Conversation
|
This pull request is being automatically deployed with Vercel (learn more). astro-www – ./www🔍 Inspect: https://vercel.com/pikapkg/astro-www/HBpt55dX6CsNQtds4jDa6ZEnP6Gk [Deployment for 2d80feb canceled] astro-docs – ./docs🔍 Inspect: https://vercel.com/pikapkg/astro-docs/41DmdcKBfDSZVEChd7fx3Kx5NBJS [Deployment for 2d80feb canceled] |
|
| - name: "Smoke Test: Build 'www'" | ||
| run: yarn build | ||
| working-directory: ./www | ||
| # NOTE: temporarily disabled until `next` branch can build docs again |
There was a problem hiding this comment.
Temporarily disabled docs build test until we get a little farther
| } | ||
|
|
||
| export const createComponent = (cb: AstroComponentFactory) => { | ||
| export function createComponent(cb: AstroComponentFactory) { |
There was a problem hiding this comment.
Minor code style nit: I think functions are generally better because they all get hoisted and don’t depend on order of definition like const does
| @@ -1,41 +0,0 @@ | |||
| import type { AstroComponent, AstroComponentFactory } from '../internal'; | |||
|
|
|||
| import { spreadAttributes, defineStyleVars, defineScriptVars } from '../internal'; | |||
There was a problem hiding this comment.
This file was deleted because this introduced a circular dependency between src/runtime/astro.ts and src/internal/index.ts. This was just moved into internal.
Naming is hard.
| async function resolveImportedModules(viteServer: ViteDevServer, file: string) { | ||
| const { url } = await viteServer.moduleGraph.ensureEntryFromUrl(file); | ||
| async function resolveImportedModules(viteServer: ViteDevServer, file: URL) { | ||
| const { url } = await viteServer.moduleGraph.ensureEntryFromUrl(slash(fileURLToPath(file))); // note: for some reason Vite expects forward slashes here for Windows, which `slash()` helps resolve |
There was a problem hiding this comment.
Fun Windows Vite bug! 🎉 New test runner caught it.
| expect($('#true').attr('type')).toBe('boolean'); | ||
| expect($('#false').attr('attr')).toBe('attr-false'); | ||
| expect($('#false').attr('type')).toBe('boolean'); | ||
| expect($('#true').attr('attr')).to.equal('attr-true'); |
There was a problem hiding this comment.
Mocha + Chai was selected because of its similarity to Jest. Migrating was just a quick find-and-replace.
|
AMAZING, nice work |
|
Nice! I've had a great experience w/ Ava in https://github.com/snowpackjs/prettier-plugin-astro - it's fully ESM in Avajs 4.0 & has a similar setup to uvu, but is much more reliable! |
Changes
In switching to Vite and having to rework tests with the new compiler, I took the opportunity to revisit our test runner. Since Jest had just released ESM support, it seemed like a good option. The biggest motivator for switching from uvu -> Jest is simply contributor familiarity; I wanted to prioritize reducing the barrier to contribute testing using what most JS devs are familiar with. But other benefits were timeouts, clearer errors, more utilities, and an overall more stable runner. Fortunately Jest worked! 🎉 … But only locally 😑. It has not been passing CI.
This PR switches to Mocha / Chai, again with the priority of “use what most people know.” This is the closest thing to Jest, and if you know Jest you probably can pick this up quickly.
Basically:
Before
After
Testing
Testing PR.
Docs