Enable Windows tests #1712
Enable Windows tests #1712
Conversation
|
58f7c4b
to
c9de0f7
Compare
| "yellow": [255, 255, 0], | ||
| "yellowgreen": [154, 205, 50] | ||
| var colorName$1 = { | ||
| "aliceblue": [240, 248, 255], |
There was a problem hiding this comment.
I have no idea what my editor did here. Ignore the noise in this file.
| @@ -46,7 +46,8 @@ export default function astro({ config, devServer }: AstroPluginOptions): vite.P | |||
| } | |||
| // pages and layouts should be transformed as full documents (implicit <head> <body> etc) | |||
| // everything else is treated as a fragment | |||
| const isPage = id.startsWith(fileURLToPath(config.pages)) || id.startsWith(fileURLToPath(config.layouts)); | |||
| const normalizedID = fileURLToPath(new URL(`file:https://${id}`)); | |||
| const isPage = normalizedID.startsWith(fileURLToPath(config.pages)) || id.startsWith(fileURLToPath(config.layouts)); | |||
There was a problem hiding this comment.
Plugin change: id was a Windows path that didn’t match up to fileURLToPath(), so basically everything in Windows was being treated as a partial.
| @@ -79,7 +79,8 @@ export async function ssr({ astroConfig, filePath, logging, mode, origin, pathna | |||
| // Important: This needs to happen first, in case a renderer provides polyfills. | |||
| const renderers = await resolveRenderers(viteServer, astroConfig); | |||
| // Load the module from the Vite SSR Runtime. | |||
| const mod = (await viteServer.ssrLoadModule(fileURLToPath(filePath))) as ComponentInstance; | |||
| const viteFriendlyURL = `/@fs${filePath.pathname}`; | |||
| const mod = (await viteServer.ssrLoadModule(viteFriendlyURL)) as ComponentInstance; | |||
There was a problem hiding this comment.
This is a bit of a hack, but it turns out Vite uses these URLs internally for Windows. TIL that this doesn’t necessarily have to be a “correct” path on disk; just a URL that Vite understands.
569ab2d
to
3ecd797
Compare
3ecd797
to
f0c3d97
Compare
| @@ -142,7 +142,11 @@ class AstroBuilder { | |||
| target: 'es2020', // must match an esbuild target | |||
| }, | |||
| plugins: [ | |||
| rollupPluginHTML({ input, extractAssets: false }) as any, // "any" needed for CI; also we don’t need typedefs for this anyway | |||
| rollupPluginHTML({ | |||
| rootDir: viteConfig.root, | |||
There was a problem hiding this comment.
This was the other fix: @web/rollup-plugin-html needs rootDir for Windows. Otherwise it tries to guess (which for some reason works on unix but fails miserably on Windows).
| @@ -21319,7 +21319,7 @@ function buildHtmlPlugin(config) { | |||
| .map((child) => child.content || '') | |||
| .join(''); | |||
| // <script type="module">...</script> | |||
| const filePath = id.replace(config.root, ''); | |||
| const filePath = id.replace(normalizePath$4(config.root), '') | |||
There was a problem hiding this comment.
Vite fix 1/2: normalize config.root
| @@ -56977,14 +56976,14 @@ const devHtmlHook = async (html, { path: htmlPath, server, originalUrl }) => { | |||
| processNodeUrl(src, s, config, htmlPath, originalUrl); | |||
| } | |||
| else if (isModule) { | |||
| const url = filePath.replace(config.root, ''); | |||
| const url = filePath.replace(normalizePath$4(config.root), ''); | |||
There was a problem hiding this comment.
Vite fix 2/2: normalize root, and also use the full filePath below
| @@ -21179,7 +21179,7 @@ function htmlInlineScriptProxyPlugin(config) { | |||
| if (proxyMatch) { | |||
| const index = Number(proxyMatch[1]); | |||
| const file = cleanUrl(id); | |||
| const url = file.replace(config.root, ''); | |||
| const url = file.replace(normalizePath$4(config.root), ''); | |||
There was a problem hiding this comment.
This was a 3rd Vite fix that technically wasn’t needed for our fix, but I’m almost positive is needed elsewhere and can’t hurt because it’s the same action.
|
lgtm, awesome work. |
Yup! Keeping track of these. We’re back down to this being our only change from Vite and I’ll open that PR on their end soon |
Changes
Enables Windows tests, fixes Windows bugs.
Current status: dev is passing in Windows! 🎉 But build seems to still have an error. So the problem is halfway solved; it just seems like there’s an extra step in build that’s failing.
Testing
Tests should pass!
Vite-wise, Vite currently doesn’t have any tests for these code paths. We could look into adding them, but it’d be tough to do without adding new compile-to-HTML examples. Will look into it after this is passing.
Docs
No docs needed