fix(ssr): change CommonJS and resolve plugin order#10450
fix(ssr): change CommonJS and resolve plugin order#10450gb-jos wants to merge 1 commit intovitejs:mainfrom
Conversation
|
greatly appreciated if one of you can take a look at it @bluwy @patak-dev |
|
Changing the order of plugins would almost always break things, and the tests are currently failing. I haven't looked deep into the issue, but I think it needs a different solution. |
|
@bluwy I did not really look into tests, because locally some were already failing for me in main branch strangely. Also not sure if it makes any sense to try to fix the CommonJS plugin, as we are anyways plan to move to dep optimiser. The issue I had with the dep optimiser was a much more straightforward one, it seemingly does not handle default exports properly. Basically with |
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
__esModuleflag 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).