Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Upgrade Vite to latest #2424

Merged
merged 25 commits into from
Feb 8, 2022
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
5 changes: 5 additions & 0 deletions .changeset/angry-apricots-invite.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
---
'astro': patch
---

Fixes HMR of CSS that is imported from astro, when using the static build flag
5 changes: 5 additions & 0 deletions .changeset/loud-seals-camp.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
---
'astro': minor
---

Upgrade `vite` to `2.8.x`, unvendoring `vite` and bringing Astro up-to-date with the ecosystem
7 changes: 7 additions & 0 deletions .changeset/many-berries-bake.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,7 @@
---
'@astrojs/renderer-svelte': minor
'@astrojs/renderer-vue': minor
'@astrojs/renderer-solid': minor
---

Upgrade to [email protected]
30 changes: 0 additions & 30 deletions examples/with-nanostores/src/components/AdminsPreact.jsx

This file was deleted.

2 changes: 0 additions & 2 deletions examples/with-nanostores/src/pages/index.astro
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,6 @@
import AdminsReact from '../components/AdminsReact.jsx';
import AdminsSvelte from '../components/AdminsSvelte.svelte';
import AdminsVue from '../components/AdminsVue.vue';
import AdminsPreact from '../components/AdminsPreact.jsx';
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is a change worth calling out... Had to drop the @nanostores/preact usage from this example to get everything passing. I think there's some issue with deduping going on, but I'm hoping it's just in our monorepo. Will be sure to check this once we cut a release.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for the callout. If you're worried about not being able to resolve this before end of week, I'd suggest creating an issue to track adding @nanostores/preact back.

import AdminsSolid from '../components/AdminsSolid.jsx';

// Full Astro Component Syntax:
Expand Down Expand Up @@ -46,7 +45,6 @@ import AdminsSolid from '../components/AdminsSolid.jsx';
<AdminsReact client:load />
<AdminsSvelte client:load />
<AdminsVue client:load />
<AdminsPreact client:load />
<AdminsSolid client:load />
</main>
</body>
Expand Down
16 changes: 7 additions & 9 deletions packages/astro/package.json
Original file line number Diff line number Diff line change
Expand Up @@ -66,18 +66,16 @@
"@astropub/webapi": "^0.10.1",
"@babel/core": "^7.15.8",
"@babel/traverse": "^7.15.4",
"@proload/core": "^0.2.1",
"@proload/core": "^0.2.2",
"@proload/plugin-tsm": "^0.1.0",
"@types/babel__core": "^7.1.15",
"@types/debug": "^4.1.7",
"@web/parse5-utils": "^1.3.0",
"astring": "^1.7.5",
"ci-info": "^3.2.0",
"common-ancestor-path": "^1.0.1",
"eol": "^0.9.1",
"es-module-lexer": "^0.9.3",
"esbuild": "0.13.7",
"estree-util-value-to-estree": "^1.2.0",
"estree-walker": "^3.0.0",
"fast-glob": "^3.2.7",
"fast-xml-parser": "^4.0.0-beta.3",
Expand All @@ -93,10 +91,11 @@
"prismjs": "^1.25.0",
"rehype-slug": "^5.0.0",
"resolve": "^1.20.0",
"rollup": "^2.57.0",
"sass": "^1.43.4",
"rollup": "^2.64.0",
"sass": "^1.49.0",
"semver": "^7.3.5",
"send": "^0.17.1",
"serialize-javascript": "^6.0.0",
"shiki": "^0.10.0",
"shorthash": "^0.0.2",
"slash": "^4.0.0",
Expand All @@ -106,7 +105,7 @@
"strip-ansi": "^7.0.1",
"supports-esm": "^1.0.0",
"tsconfig-resolver": "^3.0.1",
"vite": "~2.6.10",
"vite": "^2.8.0-beta.7",
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

No more vendored vite! It's a real dependency.

"yargs-parser": "^21.0.0",
"zod": "^3.8.1"
},
Expand All @@ -125,9 +124,8 @@
"@types/yargs-parser": "^20.2.1",
"chai": "^4.3.4",
"cheerio": "^1.0.0-rc.10",
"hast-util-select": "^5.0.1",
"mocha": "^9.1.3",
"vite": "~2.6.10"
"execa": "^6.0.0",
"mocha": "^9.1.3"
},
"engines": {
"node": "^14.15.0 || >=16.0.0",
Expand Down
3 changes: 3 additions & 0 deletions packages/astro/src/@types/serialize-javascript.d.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,3 @@
declare module 'serialize-javascript' {
export default function serialize(value: any): string;
}
7 changes: 2 additions & 5 deletions packages/astro/src/core/create-vite.ts
Original file line number Diff line number Diff line change
Expand Up @@ -16,18 +16,15 @@ import { resolveDependency } from './util.js';
const ALWAYS_EXTERNAL = new Set([
...builtinModules.map((name) => `node:${name}`),
'@sveltejs/vite-plugin-svelte',
'estree-util-value-to-estree',
'micromark-util-events-to-acorn',
'serialize-javascript',
'node-fetch',
'prismjs',
'shiki',
'shorthash',
'unified',
'whatwg-url',
]);
const ALWAYS_NOEXTERNAL = new Set([
'astro', // This is only because Vite's native ESM doesn't resolve "exports" correctly.
]);
Comment on lines -28 to -30
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This comment is no longer true, so astro can now be optimized by Vite.


// note: ssr is still an experimental API hence the type omission
export type ViteConfigWithSSR = vite.InlineConfig & { ssr?: { external?: string[]; noExternal?: string[] } };
Expand Down Expand Up @@ -72,7 +69,7 @@ export async function createVite(inlineConfig: ViteConfigWithSSR, { astroConfig,
// Note: SSR API is in beta (https://vitejs.dev/guide/ssr.html)
ssr: {
external: [...ALWAYS_EXTERNAL],
noExternal: [...ALWAYS_NOEXTERNAL],
noExternal: [],
},
};

Expand Down
4 changes: 2 additions & 2 deletions packages/astro/src/core/vite.ts
Original file line number Diff line number Diff line change
@@ -1,2 +1,2 @@
export * from '../../vendor/vite/dist/node/index.js';
export { default } from '../../vendor/vite/dist/node/index.js';
export * from 'vite';
export { default } from 'vite';
26 changes: 2 additions & 24 deletions packages/astro/src/runtime/server/hydration.ts
Original file line number Diff line number Diff line change
@@ -1,34 +1,12 @@
import type { AstroComponentMetadata } from '../../@types/astro';
import type { SSRElement, SSRResult } from '../../@types/astro';
import { valueToEstree } from 'estree-util-value-to-estree';
import * as astring from 'astring';
Comment on lines -3 to -4
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

These packages we needlessly complex and weren't being handled well by Vite.

import { hydrationSpecifier, serializeListValue } from './util.js';

const { generate, GENERATOR } = astring;

// INVESTIGATE: What features are we getting from this that we need?
// JSON.stringify has a "replacer" argument.
// A more robust version alternative to `JSON.stringify` that can handle most values
// see https://github.com/remcohaszing/estree-util-value-to-estree#readme
const customGenerator: astring.Generator = {
...GENERATOR,
Literal(node, state) {
if (node.raw != null) {
// escape closing script tags in strings so browsers wouldn't interpret them as
// closing the actual end tag in HTML
state.write(node.raw.replace('</script>', '<\\/script>'));
} else {
GENERATOR.Literal(node, state);
}
},
};
import serializeJavaScript from 'serialize-javascript';
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This was replaced by serializeJavaScript which handles everything for us.


// Serializes props passed into a component so that they can be reused during hydration.
// The value is any
export function serializeProps(value: any) {
return generate(valueToEstree(value), {
generator: customGenerator,
});
return serializeJavaScript(value);
}

const HydrationDirectives = ['load', 'idle', 'media', 'visible', 'only'];
Expand Down
20 changes: 3 additions & 17 deletions packages/astro/src/vite-plugin-astro/compile.ts
Original file line number Diff line number Diff line change
Expand Up @@ -12,23 +12,9 @@ type CompilationCache = Map<string, CompileResult>;

const configCache = new WeakMap<AstroConfig, CompilationCache>();

// https://github.com/vitejs/vite/discussions/5109#discussioncomment-1450726
function isSSR(options: undefined | boolean | { ssr: boolean }): boolean {
if (options === undefined) {
return false;
}
if (typeof options === 'boolean') {
return options;
}
if (typeof options == 'object') {
return !!options.ssr;
}
return false;
}

type CompileResult = TransformResult & { rawCSSDeps: Set<string> };

async function compile(config: AstroConfig, filename: string, source: string, viteTransform: TransformHook, opts: boolean | undefined): Promise<CompileResult> {
async function compile(config: AstroConfig, filename: string, source: string, viteTransform: TransformHook, opts: { ssr: boolean }): Promise<CompileResult> {
// pages and layouts should be transformed as full documents (implicit <head> <body> etc)
// everything else is treated as a fragment
const filenameURL = new URL(`file://${filename}`);
Expand Down Expand Up @@ -69,7 +55,7 @@ async function compile(config: AstroConfig, filename: string, source: string, vi
lang,
id: normalizedID,
transformHook: viteTransform,
ssr: isSSR(opts),
ssr: opts.ssr,
});

let map: SourceMapInput | undefined;
Expand Down Expand Up @@ -119,7 +105,7 @@ export async function cachedCompilation(
filename: string,
source: string | null,
viteTransform: TransformHook,
opts: boolean | undefined
opts: { ssr: boolean }
): Promise<CompileResult> {
let cache: CompilationCache;
if (!configCache.has(config)) {
Expand Down
13 changes: 9 additions & 4 deletions packages/astro/src/vite-plugin-astro/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -64,15 +64,20 @@ export default function astro({ config, logging }: AstroPluginOptions): vite.Plu
let { filename, query } = parseAstroRequest(id);
if (query.astro) {
if (query.type === 'style') {
if (filename.startsWith('/@fs')) {
filename = filename.slice('/@fs'.length);
} else if (filename.startsWith('/') && !ancestor(filename, config.projectRoot.pathname)) {
filename = new URL('.' + filename, config.projectRoot).pathname;
}

Comment on lines +67 to +72
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@matthewp any idea if this was old code that snuck back in via a rebase?

if (typeof query.index === 'undefined') {
throw new Error(`Requests for Astro CSS must include an index.`);
}

const transformResult = await cachedCompilation(config, normalizeFilename(filename), null, viteTransform, opts);
const transformResult = await cachedCompilation(config, normalizeFilename(filename), null, viteTransform, { ssr: Boolean(opts?.ssr) });

// Track any CSS dependencies so that HMR is triggered when they change.
await trackCSSDependencies.call(this, { viteDevServer, id, filename, deps: transformResult.rawCSSDeps });

const csses = transformResult.css;
const code = csses[query.index];

Expand All @@ -84,7 +89,7 @@ export default function astro({ config, logging }: AstroPluginOptions): vite.Plu
throw new Error(`Requests for hoisted scripts must include an index`);
}

const transformResult = await cachedCompilation(config, normalizeFilename(filename), null, viteTransform, opts);
const transformResult = await cachedCompilation(config, normalizeFilename(filename), null, viteTransform, { ssr: Boolean(opts?.ssr) });
const scripts = transformResult.scripts;
const hoistedScript = scripts[query.index];

Expand All @@ -106,7 +111,7 @@ export default function astro({ config, logging }: AstroPluginOptions): vite.Plu
}

try {
const transformResult = await cachedCompilation(config, id, source, viteTransform, opts);
const transformResult = await cachedCompilation(config, id, source, viteTransform, { ssr: Boolean(opts?.ssr) });

// Compile all TypeScript to JavaScript.
// Also, catches invalid JS/TS in the compiled output before returning.
Expand Down
19 changes: 2 additions & 17 deletions packages/astro/src/vite-plugin-jsx/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -24,21 +24,6 @@ const IMPORT_STATEMENTS: Record<string, string> = {
// be careful about esbuild not treating h, React, Fragment, etc. as unused.
const PREVENT_UNUSED_IMPORTS = ';;(React,Fragment,h);';

// This check on a flexible "options" object is needed because Vite uses this flexible argument for ssr.
// More context: https://github.com/vitejs/vite/discussions/5109#discussioncomment-1450726
function isSSR(options: undefined | boolean | { ssr: boolean }): boolean {
if (options === undefined) {
return false;
}
if (typeof options === 'boolean') {
return options;
}
if (typeof options == 'object') {
return !!options.ssr;
}
return false;
}

function getEsbuildLoader(fileExt: string): string {
return fileExt.substr(1);
}
Expand Down Expand Up @@ -103,8 +88,8 @@ export default function jsx({ config, logging }: AstroPluginJSXOptions): Plugin
configResolved(resolvedConfig) {
viteConfig = resolvedConfig;
},
async transform(code, id, ssrOrOptions) {
const ssr = isSSR(ssrOrOptions);
async transform(code, id, opts) {
const ssr = Boolean(opts?.ssr);
if (!JSX_EXTENSIONS.has(path.extname(id))) {
return null;
}
Expand Down
7 changes: 6 additions & 1 deletion packages/astro/test/0-css.test.js
Original file line number Diff line number Diff line change
Expand Up @@ -277,8 +277,13 @@ describe('CSS', function () {
expect((await fixture.fetch(href)).status).to.equal(200);
});

it('resolved imported CSS with ?url', async () => {
// Skipped until upstream fix lands
// Our fix: https://github.com/withastro/astro/pull/2106
// OG Vite PR: https://github.com/vitejs/vite/pull/5940
// Next Vite PR: https://github.com/vitejs/vite/pull/5796
it.skip('resolved imported CSS with ?url', async () => {
const href = $('link[href$="imported-url.css"]').attr('href');
expect(href).to.be.ok;
expect((await fixture.fetch(href)).status).to.equal(200);
});

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,8 @@ export default {
viteConfig() {
return {
optimizeDeps: {
include: ['@astrojs/test-custom-element-renderer/polyfill.js', '@astrojs/test-custom-element-renderer/hydration-polyfill.js']
include: ['@astrojs/test-custom-element-renderer/polyfill.js', '@astrojs/test-custom-element-renderer/hydration-polyfill.js'],
exclude: ['@astrojs/test-custom-element-renderer/server.js']
}
}
}
Expand Down
8 changes: 4 additions & 4 deletions packages/astro/test/fixtures/static build/pkg/package.json
Original file line number Diff line number Diff line change
@@ -1,11 +1,11 @@
{
"name": "@astrojs/test-static-build-pkg",
"main": "./oops.js",
"version": "0.0.2",
"main": "./oops.cjs",
"version": "0.0.3",
"exports": {
".": {
"import": "./pkg.js",
"require": "./oops.js"
"import": "./pkg.mjs",
"require": "./oops.cjs"
}
}
}
Loading