-
-
Notifications
You must be signed in to change notification settings - Fork 6.2k
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
fix(ssr): avoid transforming json file in ssrTransform #6597
Conversation
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.
In the transform phase of the plugin, dataToEsm is called for tree shaking, but I think that in the runtime phase, there should be no need to shake the json tree, and it may directly return a string. I am not sure whether this point needs to be changed.
Good point. I think in dev it's fine to not treeshake as bundle size is not a concern, plus the issues showed that Node parses it fairly faster as well. In build we can treeshake the JSON (which I believe is handled separately).
Also, the PR title ssrTransfrom
-> ssrTransform
😬
@bluwy I found out why I need to call datatoesm. Not only for tree sharking, but also for named exports I think we can only optimize the situation where named exports is not required. 😂 |
I think we missed out on merging this. It looks good to me, but the PR now needs a rebase. |
done. the unit test meeting a common mistake. |
Description
fix: #6154
Additional context
`filter json file transform in ssrTransform.need to filter json tree shaking in runtime phase?In the transform phase of the plugin,dataToEsm
is called for tree shaking, but I think that in the runtime phase, there should be no need to shake the json tree, and it may directly return a string. I am not sure whether this point needs to be changed.feat json ignore named export option in ssr.
What is the purpose of this pull request?
Before submitting the PR, please make sure you do the following
fixes #123
).