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

Upgrade Vite to latest #2424

merged 25 commits into from
Feb 8, 2022

Conversation

natemoo-re
Copy link
Member

@natemoo-re natemoo-re commented Jan 19, 2022

Changes

Testing

Tracking tests in CI

Docs

N/A

@changeset-bot
Copy link

changeset-bot bot commented Jan 19, 2022

🦋 Changeset detected

Latest commit: 7d1479b

The changes in this PR will be included in the next version bump.

This PR includes changesets to release 4 packages
Name Type
astro Minor
@astrojs/renderer-svelte Minor
@astrojs/renderer-vue Minor
@astrojs/renderer-solid Minor

Not sure what this means? Click here to learn what changesets are.

Click here if you're a maintainer who wants to add another changeset to this PR

@github-actions github-actions bot added pkg: astro Related to the core `astro` package (scope) pkg: solid Related to Solid (scope) pkg: svelte Related to Svelte (scope) pkg: vue Related to Vue (scope) pkg: integration Related to any renderer integration (scope) test labels Jan 19, 2022
@natemoo-re natemoo-re marked this pull request as draft January 19, 2022 23:38
@netlify
Copy link

netlify bot commented Jan 19, 2022

✔️ Deploy Preview for astro-docs-2 ready!

🔨 Explore the source changes: 56f75c1

🔍 Inspect the deploy log: https://app.netlify.com/sites/astro-docs-2/deploys/61f8553d3a613b000876b20a

😎 Browse the preview: https://deploy-preview-2424--astro-docs-2.netlify.app

@natemoo-re natemoo-re changed the title Upgrade to [email protected] Upgrade Vite to latest Jan 19, 2022
@matthewp
Copy link
Contributor

Blocked by vitejs/vite#6576

@matthewp
Copy link
Contributor

This is blocked by another Vite bug that I'm submitting a PR for shortly.

@matthewp
Copy link
Contributor

Blocked by vitejs/vite#6589

@natemoo-re
Copy link
Member Author

Looks like the final issues caused by Svelte are now passing! Just two issues left.

@jonathantneal jonathantneal changed the title Upgrade Vite to latest [WIP] Upgrade Vite to latest Jan 25, 2022
@natemoo-re natemoo-re force-pushed the chore/upgrade-vite branch 3 times, most recently from 228f74a to 56f75c1 Compare January 31, 2022 21:31
@natemoo-re
Copy link
Member Author

This is super close, one last absolute path issue on Windows.

@github-actions github-actions bot added the pkg: example Related to an example package (scope) label Feb 8, 2022
@natemoo-re natemoo-re marked this pull request as ready for review February 8, 2022 21:37
@natemoo-re natemoo-re changed the title [WIP] Upgrade Vite to latest Upgrade Vite to latest Feb 8, 2022
@@ -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.

@@ -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.

Comment on lines -28 to -30
const ALWAYS_NOEXTERNAL = new Set([
'astro', // This is only because Vite's native ESM doesn't resolve "exports" correctly.
]);
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.

Comment on lines -3 to -4
import { valueToEstree } from 'estree-util-value-to-estree';
import * as astring from 'astring';
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 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.

Comment on lines +67 to +72
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;
}

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?

Copy link
Member

@FredKSchott FredKSchott left a comment

Choose a reason for hiding this comment

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

LGTM! What a huge effort @natemoo-re !

Just a callout that this will be released as npm install astro@next before it goes out to everyone, so there will be time to test with real-user data post-merge.

Copy link
Contributor

@jonathantneal jonathantneal left a comment

Choose a reason for hiding this comment

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

For what this change is, it was surprising clear. Well done, @natemoo-re!

@natemoo-re natemoo-re merged commit 1abb9ed into main Feb 8, 2022
@natemoo-re natemoo-re deleted the chore/upgrade-vite branch February 8, 2022 23:38
@github-actions github-actions bot mentioned this pull request Feb 8, 2022
@github-actions github-actions bot mentioned this pull request Feb 18, 2022
SiriousHunter pushed a commit to SiriousHunter/astro that referenced this pull request Feb 3, 2023
* chore: unvendor vite

* chore: upgrade vue and svelte

* feat(renderer-solid): upgrade for [email protected]

* chore: update yarn.lock

* fix(solid): upgrade solid renderer for [email protected]

* test: improve css test

* chore: upgrade to [email protected]

* fix: replace hacky serialization with 'serialize-javascript'

* fix: externalize serialize-javascript

* fix: explicitly add [email protected] to devDependencies

* test(css): skip css?url test

* chore: update vite-plugin-svelte

* fix: ssr option

* chore: update changeset

* chore: remove changeset

* chore: add changeset

* chore: add back missing changeset

* chore: update vite

* chore: update to latest vite

* test: update proload?

* chore: update dependencies

* fix: remove preact from nanostores example

* fix: update static-build-pkg to use `.mjs` extension

* fix: exclude './server.js' from custom-elements test

* chore: remove unused file
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
pkg: astro Related to the core `astro` package (scope) pkg: example Related to an example package (scope) pkg: integration Related to any renderer integration (scope) pkg: solid Related to Solid (scope) pkg: svelte Related to Svelte (scope) pkg: vue Related to Vue (scope)
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants