-
-
Notifications
You must be signed in to change notification settings - Fork 6.4k
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
chore: upgrade rollup to 3.25.2 #13608
Conversation
Run & review this pull request in StackBlitz Codeflow. |
It seems that rollup version upgrade logic didn't follow the semantic versioning. I was thinking whether vite should keep the rollup version in the published package the same as vite |
/ecosystem-ci run |
📝 Ran ecosystem CI: Open
|
For reference, we already did this for a while last year, after other type changes in a rollup minor brought issues downstream. Since it is rare, we ended up reverting with time so projects could use newer versions of rollup sooner. It is hard to know what is best here. I'm leaning towards keeping the current scheme, interested in others' opinions. |
Yeah I think it's probably fine to keep it as-is for now. We have close development with Rollup, so we can followup the type changes as needed. The PR looks good though. Could you also update it to 3.25.2 to fix #13571? Also, when I run |
@@ -15,7 +15,7 @@ export function prepareError(err: Error | RollupError): ErrorPayload['err'] { | |||
id: (err as RollupError).id, | |||
frame: strip((err as RollupError).frame || ''), | |||
plugin: (err as RollupError).plugin, | |||
pluginCode: (err as RollupError).pluginCode, | |||
pluginCode: (err as RollupError).pluginCode?.toString(), |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Not sure whether we should deal with non-(string|number)
-type pluginCode
and code
.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think that's fine for now 👍
All failing CIs are also failing in main, except for vite-plugin-ssr. @brillout I think we should merge this PR and release a new beta so we give a week more to others to try it out. I'll do that now, would you check why updating rollup is affecting your tests? |
@patak-dev Fix pushed on VPS's But I get another error which seems unrelated to Rollup:
The error originates from: // node_modules/vite/dist/node/chunks/dep-64acfab2.js:65812:19
65798 async function loadConfigFromBundledFile(fileName, bundledCode, isESM) {
65799 // for esm, before we can register loaders without requiring users to run node
65800 // with --experimental-loader themselves, we have to do a hack here:
65801 // convert to base64, load it with native Node ESM.
65802 if (isESM) {
65803 try {
65804 // Postfix the bundled code with a timestamp to avoid Node's ESM loader cache
65805 const configTimestamp = `${fileName}.timestamp:${Date.now()}-${Math.random()
65806 .toString(16)
65807 .slice(2)}`;
65808 return (await dynamicImport('data:text/javascript;base64,' +
65809 Buffer.from(`${bundledCode}\n//${configTimestamp}`).toString('base64'))).default;
65810 }
65811 catch (e) {
65812 throw new Error(`${e.message} at ${fileName}`);
65813 }
65814 } Let me know if you want a reproduction for that one. |
Yes, please create a new issue. We're trying to avoid the temporal bundled config file in 4.4 |
Done: #13631. |
Description
Adjust rollup log API calls
Adjust rollup error code
Delay removing the independent inline entry chunk in MPA since
[email protected]
would share entry chunks. ps: vite already contains the test cases inplayground/html
fix Internal rollup crash when transform generates invalid js #13571
Additional context
What is the purpose of this pull request?
Before submitting the PR, please make sure you do the following
fixes #123
).