-
Notifications
You must be signed in to change notification settings - Fork 36
[discussion] Tree shake-ability #138
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
Comments
Also webpack4 will look at |
I'm aware of the |
Hi @Andarist, thanks for raising this issue! I'm in favor of anything that helps, as long as we feel the concrete benefit to users outweighs any additional maintenance costs or complexity. From that perspective, it seems like For the PURE annotations, buble satisfies our needs, and we are close to 1.0, so I'd rather not make a major build system change right now. I'm open to revisiting that after 1.0, though. Buble does tend to generate leaner code than babel in the first place. I wonder if the tradeoff of adding babel's overhead, but with improved tree-shaking turns out to yield better results for users on average. Do you have any intuition about that? Would it be possible to have a rollup version of the plugin? Or is there some way to use the existing plugin with rollup, without fully committing to babel right now? |
That's really hard to measure, for sure PURE helps - although it is kinda a hack. According to projects using those it can help quite a lot, but also in kinda a specific situations - angular projects are heavily class-based (which ends to be IIFEs when transpiled). Also it helped a lot in ramda's case, which is heavily curry-based - just like the mostjs. Truth to be told I'm closely looking into those matters lately and it doesnt seem to be 1 perfect solution. We have also started adding
Agreed, but I think babel's loose mode doesn't add too much overhead over buble's output. Would have to be compared though. It is also not impossible (and even rather easy) to write custom babel transforms for cases when one would want to have even looser output (i.e. similar to buble's).
That was my first intuition, but it seems that buble doesn't support plugins, so the only way to add this without commiting to babel would be to create some rollup plugin - aint sure how much work would that be. And ofc there is always an option to use babel only for this single transform - then it should be a matter of adding the babel plugin to rollup's config. |
The problem is that Babel breaks code style because of code generator. Buble saves it in most cases by nature. I can make plugin. Is there a link about pure annotaton? |
@TrySound that would be interesting to see theres a mention of it here from the link the OP shared: https://iamakulov.com/notes/polished-webpack/#solution and some more links and discussions here babel/babel#5632 |
@Andarist Did you see similar helper in babel repo? |
Yeah, I've created it recently :P It's purpose is different though. It's just a helper which annotates node or path (+ checking if its already annotated - to prevent duplicated annotations). There is no decision involved in the process. (also it's only available since like recent babel7 beta releases) My annotate-pure-calls plugin visits each CallExpression and NewExpression and check if their results are used in assignment contexts and if they are at the top level (for regular cases this means a program's level) - if both conditions are met, the plugin assumes they are pure and annotates them. You can check the Readme and the tests to get better overview. |
I created a quick proof of concept over in #140 which does that. Thoughts? |
Ok for me, but you guys have mentioned that you care about output formatting (which can get screwed up by babel generator). So you would have to compare pre and post outputs in regards of formatting. |
@TrySound Can you explain more what you mean by "Babel breaks code style because of code generator"? |
Code looks differently. Buble replaces parts of code traversing ast, when babel generates string from ast. It's makes buble output nicer, but makes hard to process all language features and combinations, also introduces a few unexpected bugs which are hard to maintain. Babel is safer, community driven and covers all (and even more) language features. Talking as buble collaborator in the past :) |
It seems like you're saying there are 2 potential risks:
Am I understanding correctly? While I do cringe at the added complexity of having both buble and babel in the codebase, we'd be using only a single, focused, babel transform, whose only effect is to insert comments in certain spots. So, we're going from 100% buble output to 99.9% buble output plus pure annotation comments. That seems reasonably safe, but since I've never done it before, I can't speak to it in practice. How risky is it in your view? |
I meant that using buble and babel together is overtooling and you won't get most of benefits of buble with babel after it and buble is not quite safe tool. |
Are you saying that babel changes the built output (and not just comments), possibly increasing the size and/or complexity of the built JS, even if we're only using the pure annotation comment plugin? If so, then yeah, that seems like a very valid concern. |
I think the concern is different. It is purely about the fact that babel uses such approach: which in other words is: I think buble + babel can work together, although as mentioned it will prevent (in any configuration used) buble to preserve code style of the input, because it will be parsed with babel too (no matter in which order) which can alter the code style, because it uses its own "style" when generating the output. Babel won't apply any other transforms than requested, running it without any plugins/presets would result in the very same code (maybe just a little bit reformatted). |
See this comment. I'm sure I'm missing something. Perhaps someone who knows these tooling intricacies better than I do would be willing to put together a proof of concept that shows how to make all the things in my comment work. I think that'd help us to evaluate the best way to move forward. FWIW, I feel ok about releasing 1.0 without pure annotations, while continuing to discuss this. |
Now that PR #143 is merged, should this issue still be kept open? |
Closed by #143 |
At the current state the library, although highly modular (👏), cannot be effectively tree-shaken (well, dead code eliminated to be more exact), because of the heavy
curry
usage. It is like that because bundlers/minifiers cannot know for sure that the call tocurry
is pure and wont cause any side-effects - they cannot be sure that it is safe to remove that call, even if its result stays unused.UglifyJS has introduced possibility to mark calls as pure with comment annotation -
#__PURE__
. This could be leveraged in the library.While
#__PURE__
comments are useful today, it's quite ugly to put them into the source code and also it's error-prone, it's easy to forget to put them everywhere during development. I've created a babel plugin which helps with that and automates this task, but at the moment you are using buble and not babel for es2015+ transpilation and Im not sure if you are open to a change and if you want your code to get annotated (in any way, doesnt have to be with a plugin I've created).So in general - what do you think about this? Would you like me to send a PR with buble -> babel + the plugin?
More reading about the problem - here
The text was updated successfully, but these errors were encountered: