search

emberjs / ember-cli-babel

Ember CLI plugin for Babel
MIT License
153 stars 119 forks source link

Properly handle class properties proposal #420

Closed chriskrycho closed 2 years ago

chriskrycho commented 2 years ago

Historically, we only add the @babel/plugin-proposal-class-properties so that we make sure the ordering is right with the decorators proposal (otherwise, it can end up compiling in the wrong order). With a recent version of @babel/preset-env and, transitively, caniuse-lite, this resulted in cases where we added that plugin but not related plugins for private class properties, which in turn triggered a Babel assertion about not adding the properties together as appropriate when the caniuse database (correctly) reported that .

The fix is:

  1. Bump to a more recent version of @babel/preset-env, which comes with a correspondingly bumped version of caniuse-lite, which in turn correctly understands what the latest versions of targeted browsers are.

  2. Include in ember-cli-babel itself a check for whether we even need to add the plugin, and only provide it when the provided targets indicate that they require it.

Resolves #419

chriskrycho commented 2 years ago

Tested this, besides the unit tests, by doing a yarn pack and directly referenced the resulting .tgz and confirmed it works in do-it-break. You should also be able to use the PR by GitHub reference and confirm it works, and you can also check out the latest version of do-it-break and see that it is now fixed there by referencing this commit.

NullVoxPopuli commented 2 years ago

I confirm that with this branch, things work as expected!

image image

I expect some perf / build improvements from this! let's goooooo :ship:

nice work @chriskrycho <3

rwjblue commented 2 years ago

Rebased / amended to kick off a CI run

chriskrycho commented 2 years ago

nice work @chriskrycho <3

Totally a team effort with @rwjblue – we spent a half day pairing on it till we had a solution; I just did the "write some tests and open a PR" bit. ;)

NullVoxPopuli commented 2 years ago

Thanks, @rwjblue !!!!! <3

I super excited to deploy these updates everywhere!!!