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

Babel minor upgrade introduce inline helpers #9297

Closed
pascalduez opened this Issue Jan 8, 2019 · 6 comments

Comments

Projects
None yet
4 participants
@pascalduez
Copy link

pascalduez commented Jan 8, 2019

Bug Report

Current Behavior
Upgraded our preset dependencies to the latest minor, so from 7.1.6 to 7.2.3.
In our snapshot tests I noticed a huge amount of inline helpers added.

I couldn'f find a real hint or help from the changelog, although I understand quite some stuff changed at the class, class-properties level. But hey, it's a minor...

Input Code

Please see this MR for the diffs.
Our preset setup.

Environment

  • Babel version(s): 7.2.3
  • Node version:10.12.0
  • npm version:6.5.0
  • OS: Ubuntu Linux
  • Monorepo: no
  • How you are using Babel: [api, cli, loader]

Possible Solution
Switching order of plugin, seems to bring back the same compilation output.
But I'm not sure this is the desired output?

So from

let plugins = [
    '@babel/plugin-syntax-dynamic-import',
    '@babel/plugin-proposal-class-properties',
    ['@babel/plugin-proposal-decorators', { legacy: true }],
    ['@babel/plugin-transform-runtime', runtimeOpts],
];

to

let plugins = [
    '@babel/plugin-syntax-dynamic-import',
    ['@babel/plugin-proposal-decorators', { legacy: true }],
    '@babel/plugin-proposal-class-properties',
    ['@babel/plugin-transform-runtime', runtimeOpts],
];

Input

@decorated
class A {}

output

"import _classCallCheck from \\"@babel/runtime-corejs2/helpers/esm/classCallCheck\\";

var _class;

var A = decorated(_class = function A() {
  _classCallCheck(this, A);
}) || _class;"
@babel-bot

This comment has been minimized.

Copy link
Collaborator

babel-bot commented Jan 8, 2019

Hey @pascalduez! We really appreciate you taking the time to report an issue. The collaborators
on this project attempt to help as many people as possible, but we're a limited number of volunteers,
so it's possible this won't be addressed swiftly.

If you need any help, or just have general Babel or JavaScript questions, we have a vibrant Slack
community
that typically always has someone willing to help. You can sign-up here
for an invite.

@loganfsmyth

This comment has been minimized.

Copy link
Member

loganfsmyth commented Jan 12, 2019

Would you be able to make a small self-contained example?

@pascalduez

This comment has been minimized.

@nicolo-ribaudo

This comment has been minimized.

Copy link
Member

nicolo-ribaudo commented Jan 12, 2019

As stated in the docs (https://babeljs.io/docs/en/babel-plugin-proposal-decorators#note-compatibility-with-babel-plugin-proposal-class-properties), if you are using legacy decorators that plugin must come before the class-properties plugin: that is why the second configuration work. You was lucky and it worked, but it could have broken even without updating your Babel packages.

Probably we should throw an error instead of transforming decorators following the current spec, that is what happened with your setup.

EDIT: Actually it looks like we should already be throwing in this case, I will investigate.

@nicolo-ribaudo

This comment has been minimized.

Copy link
Member

nicolo-ribaudo commented Jan 12, 2019

A side note: by default, the code generated by transform-runtime needs to be compatible with @babel/runtime@^7.0.0: for this reason, if we create a new helper it will be directly injected in your code, without importing it. You can avoid it by specifying the version of @babel/runtime that you are using:

["@babel/transform-runtime", { "version". "7.2.0" }]
@pascalduez

This comment has been minimized.

Copy link

pascalduez commented Jan 12, 2019

@nicolo-ribaudo Thanks a bunch for the explanations!
Yeah somehow I missed the plugins documentation while upgrading to Babel 7 :\

pascalduez added a commit to Gandi/babel-preset-gandi that referenced this issue Jan 12, 2019

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment