Avoid using `@babel/runtime` as a dependency
theoludwig opened this issue · 8 comments
Describe the bug
While it is expected that tailwind-merge increase bundle size (as said in the docs) and has a cost at runtime, we should minimze it nonetheless.
We should be able to use modern JavaScript syntax and features, as supported by modern browsers, hence, I don't think adding @babel/runtime (for better compatibility? as I understand it) is worth it.
To Reproduce
npm install tailwind-merge
Expected behavior
No dependencies.
Environment
tailwind-merge@2.3.0
Hey @theoludwig! 👋
tailwind-merge is bundled to be compatible with the browserslist query > 0.5%, last 2 versions, Firefox ESR, not dead, maintained node versions (code) which is the default for most apps.
Do you have a use case where speed matters so much more than compatibility with enough browsers? The tradeoff with not transpiling at all to shave off a few bytes is that the code will break the app for some 10-20% of users.
Hey @dcastil, thanks for your reply!
I understand the desire to maintain broad compatibility, but I would like to present a case for why we should avoid including the @babel/runtime dependency in the tailwind-merge library.
The tradeoff with not transpiling at all to shave off a few bytes is that the code will break the app for some 10-20% of users.
Maybe, but then, for 80-90% of users, they are downloading and executing more JavaScript, than necessary.
Also, I'm not convinced, that removing @babel/runtime, it ill break for 10-20% users, it really depends of the JavaScript features used by tailwind-merge.
As you said: "bundled to be compatible with the browserslist query", "which is the default for most apps.", that's exactly the point, it's not the library developer responsibility, but the app developer responsibility to bundle/polyfill/transpile accordingly to what browsers they want to target. Most frameworks for apps, already handle it, whether it is Vite.js, Next.js or others.
For reference, some comments/articles, that discuss this:
- w3ctag/polyfills#6 (comment)
- https://www.builder.io/blog/stop-polyfilling-fetch-in-your-npm-package
- https://www.reddit.com/r/javascript/comments/purc3r/askjs_best_practices_for_polyfills_in_libraries/
Conclusion
In conclusion, while ensuring compatibility is important, it should not come at the cost of performance and maintainability for the majority of users. By not including @babel/runtime and allowing application developers to handle polyfills, we can create a more efficient and maintainable library that performs optimally in modern environments.
Looking forward to your thoughts on this!
That argument makes a lot of sense to me, thanks for bringing it up!
I won't be able to stop transpiling the default bundle because that could break a lot of apps. But I could add another bundle, e.g. esnext that is only converted to JS without adding any polyfills. You would then need to use it like this in your code:
import { twMerge } from 'tailwind-merge/esnext'There is already a similar bundle like that for people that need the code to be ES5-compatible (#286, #344):
import { twMerge } from 'tailwind-merge/es5'Would that be something that could work for you?
Yes, possibly, a 'tailwind-merge/esnext'. 😄
And the dependency @babel/runtime can't be removed?
That bundle wouldn't use @babel/runtime then. That's what I expect. 🤔 So when you import tailwind-merge/esnext, the Babel runtime wouldn't be in your import tree.
@theoludwig just checked the bundle and the normal tailwind-merge bundle doesn't even import @babel/runtime. You'll find the ES module in node_modules/tailwind-merge/dist/bundle-mjs.mjs and can see that there are no imports. It's only imported in the ES5 bundle in node_modules/tailwind-merge/dist/es5/bundle-mjs.mjs.
But I can see how having @babel/runtime as a dependency of tailwind-merge just for the ES5 bundle is annoying. I'll remove the dependency and inline the transform functions into the ES5 bundle since that isn't optimized for code size anyway.
This was addressed in release v2.4.0.