Open Delagen opened 4 years ago
@josephfrazier, is this the right fix?
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 https://github.com/slevithan/xregexp/pull/280 working, it should remove the deprecation, without potentially breaking anything.
I did some git blame .babelrc
and git log -S '@babel/plugin-transform-runtime'
and found that @babel/plugin-transform-runtime
was added in https://github.com/slevithan/xregexp/pull/255 to fix IE compatibility (https://github.com/slevithan/xregexp/issues/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.
That doesn’t make sense. The runtime is just sharing shortcuts/helpers that babel used to replace es6+ syntax. It’s not a polyfill. If you configure babel to use builtins or turn them off... your code is no more or less compatible with IE.
Honestly, the best solution would be to ship your esm code as-is, in parallel, under a module entry. So those of us using it with esm/webpack/rollup wouldn’t hit the transpiled code at all. (you still should not depend on the babel runtime if you can help it)
On Wed, Jan 29, 2020 at 5:56 PM Joseph Frazier notifications@github.com wrote:
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 https://github.com/slevithan/xregexp/pull/255 to fix IE compatibility (
254 https://github.com/slevithan/xregexp/issues/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.
— You are receiving this because you commented. Reply to this email directly, view it on GitHub https://github.com/slevithan/xregexp/pull/275?email_source=notifications&email_token=AAEM7LNVIGCKZ7HATRA7ZA3RAIJRZA5CNFSM4J5DM36KYY3PNVWWK3TUL52HS4DFVREXG43VMVBW63LNMVXHJKTDN5WW2ZLOORPWSZGOEKJGKAY#issuecomment-580019459, or unsubscribe https://github.com/notifications/unsubscribe-auth/AAEM7LMR5AW4Z3PFQEIWWLDRAIJRZANCNFSM4J5DM36A .
@josephfrazier It does not remove IE support, it just include some parts of runtime in code, so no runtime needed at all.
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 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 https://github.com/slevithan/xregexp/issues/259#issuecomment-450537120, where a Symbol-related issue was fixed by adding the runtime.
Sorry for the delay here, btw. It's been a crazy year.
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.
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