-
-
Notifications
You must be signed in to change notification settings - Fork 775
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
esm release tree shake is not pure #631
Comments
I'm not familiar with details. Could you explain where to read and why functions should be marked manually (that looks strange)? |
Google does not shows too much. This is interesting to read: d3/d3#3076 (comment) Also:
|
rollup pure Both 2 seems equal. |
Scenario
Plan A: pure comment
Plan B: sideEffects
|
@loynoir Sorry for delay with answer. I have no principal objections against marking required methods with comments, as you suggested. Two more questions prior to move forward (sorry, if stupid, i'm not familiar with topic):
|
@puzrin I have some thinks, but no guarantee if they are right.
I think ESM is stateless to make tree-shaking happens , so any might be un-stateless code are kept outside that tree. Eg, toplevel function call. Line 410 in 01d91e5
Maybe convert stateful function calls to stateless init functions, Or just keep multiple I don't know if there is best practice. But I'm sure I saw it in some source code, but not generated code, that's how I know this magic comment.
echo 'export {} from "./dist/js-yaml.mjs"' | rollup Besides, I add a test. Line 23 in 01d91e5
Just see the command line output. Those stateful blocks are kept in output. Besides, remember the location of those stateful thins, and check the output. |
For short, I think
|
Reproduce
Expected
output only LF
Actual
output 1023 lines
The text was updated successfully, but these errors were encountered: