search

emberjs / ember-cli-babel

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

Fix decorators babel plugin issue #426

Closed chriskrycho closed 2 years ago

chriskrycho commented 2 years ago

Starts by introducing a failing test case for #423, #425. Will also introduce more, and hopefully will fix this. Wheeeeeee.

Current working theory, based on reading the code for the Babel decorators plugin: we must actively, manually supply all the class properties (including private class properties and methods) plugins to make sure it works, since we're manually opting ourselves into decorators support.

If that's true, our fix will look like:

NullVoxPopuli commented 2 years ago

I'm curious why this "Works" then: https://github.com/NullVoxPopuli/babel-transpilation-tests

export default {
  presets: ['@babel/preset-env'],
  plugins: [['@babel/plugin-proposal-decorators', { legacy: true }]]
};

babel hurts my head lol

rwjblue commented 2 years ago

I'm curious why this "Works" then: NullVoxPopuli/babel-transpilation-tests

Mentioned in discord, but I'll also mention it here: that repro actually demonstrates the failure!!!

See this assertion: https://github.com/NullVoxPopuli/babel-transpilation-tests/blob/9bbc2c8353e45bebae9ae16defd40830614718e1/output.js#L20

NullVoxPopuli commented 2 years ago

Mentioned in discord, but I'll also mention it here: that repro actually demonstrates the failure!!!

I'm sad to see this! lol. I hadn't ran that code, because I figured it was small enough to "just be right" -- focusing primarily on the non-transpilation of the class properties, and the "supposed" transpilation of the decorators. Looking at now (it's been a while since I made that repo), it's very obvious that the problem is the same as what we saw with ember-cli-babel 7.26.7 :(

NullVoxPopuli commented 2 years ago

Just tested this out, and it's an improvement! image I no longer see defineProperty, so this is a huge improvement! thank you both <3

(previously): image