Skip to content
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

feat(babel): remove babel helper from dependencies as core-js2 outdated #275

Open
wants to merge 2 commits into
base: master
Choose a base branch
from

Conversation

Delagen
Copy link

@Delagen Delagen commented Dec 19, 2019

No description provided.

@jsg2021
Copy link
Contributor

jsg2021 commented Jan 10, 2020

This would be a very welcome change. I'm currently pinning to 4.2.0 so I don't incur the additional @babel/runtime-corejs2 dep

@slevithan
Copy link
Owner

@josephfrazier, is this the right fix?

@josephfrazier
Copy link
Collaborator

I'm not sure, I need to go back and see why we're using the helpers in the first place. If I can get #280 working, it should remove the deprecation, without potentially breaking anything.

@josephfrazier
Copy link
Collaborator

I did some git blame .babelrc and git log -S '@babel/plugin-transform-runtime' and found that @babel/plugin-transform-runtime was added in #255 to fix IE compatibility (#254), so I wouldn't recommend removing it at the moment. If we had more comprehensive browser testing (perhaps based on market share), we could get a better idea of what XRegExp browser compatibility looks like and how PRs like this would affect it.

@jsg2021
Copy link
Contributor

jsg2021 commented Jan 30, 2020 via email

@Delagen
Copy link
Author

Delagen commented Feb 12, 2020

@josephfrazier It does not remove IE support, it just include some parts of runtime in code, so no runtime needed at all.

@jsg2021
Copy link
Contributor

jsg2021 commented Feb 12, 2020

The current release make this a little less painful but I think the babel runtime should not be a dep. This change would accomplish that. However, #282 would also accomplish this AND simplify this package some. Legacy commonjs and script-tag consumers get a self-contained bundle and module-aware consumers get the original source.

josephfrazier pushed a commit that referenced this pull request Feb 13, 2020
This will allow users to consume the original esm modern code. This would make #275 a minor non-blocking issue. :)
@josephfrazier
Copy link
Collaborator

@josephfrazier It does not remove IE support, it just include some parts of runtime in code, so no runtime needed at all.

If IE does not support Symbol, then I believe the runtime is needed. See #259 (comment), where a Symbol-related issue was fixed by adding the runtime.

Sorry for the delay here, btw. It's been a crazy year.

@jsg2021
Copy link
Contributor

jsg2021 commented Jan 9, 2021

babel-runtime doesn't add Symbols. If you target IE, babel will compile symbol references and symbol consuming structures into using core-js' apis. This means you will depend on core-js.

Honestly, imo, you should probably drop all this and just publish your code as modern esm (or commonjs). Make your consumers responsible for compiling down to legacy targets.

You can still provide a umd script that's pre-built and bundled ... the tools and deps needed for that can be devDeps... that won't require external dependencies.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants