fix(ssr): change CommonJS and resolve plugin order #10450
Closed
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Description
When trying to build a node library with the ssr mode, vite was failing with couple of issues
CommonJS modules were not always transformed. The specific case for me was dependencies that are CJS build of ES module codebase with
__esModule
flag set, in this case vite ended up assuming that files in the dependency imported by files in the dependency itself are ES modules instead of CJS. Applying the CommonJS plugin earlier fixed this issueTree shaking was just not working, I got ~34MB builds and the build did not reference imports properly and was broken. Apply the resolve plugin later fixed this issue, the build came down to ~5MB and generated build was correct.
I figured this out by testing bundling by just using rollup and rollup plugins, there also I was able to reproduce the issue when the CommonJS plugin is not applied early and resolve (the rollup one, not vite one, though I guess similar) is not applied later.
Just as a side note, I did try, enabling the dep optimiser and disabling CommonJS, which did solve the two issues above, but I had the issue that default exports from node builtins were not working. I have a minimal repro of that in this repo https://github.com/jiby-aurum/vite-issue. You will be able to repo it by cloning and running
npm i && npx vite build && node dist/index.mjs
. However I did not report an issue with it, as its experimental and not supported at the moment anyways.Additional context
I am very new to the vite codebase and I am not sure if the order change of plugins is going to break something else. If it won't I would really appreciate if we can land this in 3.2 which is the next release in the pipeline
What is the purpose of this pull request?
Before submitting the PR, please make sure you do the following
fixes #123
).