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

Class properties work differently with transform-decorators-legacy #17

Open
mattinsler opened this issue Feb 3, 2016 · 21 comments
Open

Comments

@mattinsler
Copy link

Turning on transform-decorators-legacy will cause errors if you use class properties in a specific way.

class Foo {
  static SECOND = 1000;
  static MINUTE = Foo.SECOND * 60;
}

This will say that Foo is undefined while trying to evaluate Foo.SECOND. Without the decorators-legacy plugin this works.

@loganfsmyth
Copy link
Owner

@jeffmo Would you expect the above code sample to work, or is this actually my plugin doing this properly and Babel 6 doing it wrong?

https://github.com/jeffmo/es-class-fields-and-static-properties defines properties as being evaluated before classScopeEnvRec.InitializeBinding(className, F) on step 26.i so it seems like the Foo binding in this code sample would still be uninitialized when Foo.SECOND was accessed, leading to a TDZ error in a real environment and an undefined value in this case.

@jayphelps
Copy link
Contributor

jayphelps commented May 31, 2016

@mattinsler If you're manually specifying your plugins, the order sometimes matter. Make sure transform-decorators-legacy comes before transform-class-properties.

WRONG!!

"plugins": [
  "transform-class-properties",
  "transform-decorators-legacy"
]

RIGHT

"plugins": [
  "transform-decorators-legacy",
  "transform-class-properties"
]

@uMaxmaxmaximus
Copy link

uMaxmaxmaximus commented Jun 23, 2016

You'll Fix a BUG or I fixed a it for you?

@jayphelps
Copy link
Contributor

@uMaxmaxmaximus did you make sure the order of your plugins is correct, that transform-decorators-legacy comes before transform-class-properties? #17 (comment)

@uMaxmaxmaximus
Copy link

uMaxmaxmaximus commented Jun 23, 2016

yes, babel cmd --plugins transform-decorators-legacy,transform-class-properties

image

image

Generated code

"use strict";

var _class, _temp;

function _classCallCheck(instance, Constructor) {
    if (!(instance instanceof Constructor)) {
        throw new TypeError("Cannot call a class as a function");
    }
}

var User = (_temp = _class = function User() {
    _classCallCheck(this, User);
}, _class.dd = {
    lol: User
}, _temp);


console.log(User.dd.lol);

//# sourceMappingURL=user.js.map

do like this:

var User = (function () {

    function User() {
        // constructor code
    }

    // static decorator magic
    _addDecorator(User, 'dd', decoratorOptions, /* original value */ {
        lol: User // in scope
    })

    // instance decorator magic
    _addDecorator(User.prototype, 'func', decoratorOptions, /* original value */ function(){

    })

})

@loganfsmyth
Copy link
Owner

As I mentioned above, this is currently the specced behavior for this because User has not finished being defined yet when you are attempting to access it. In a real ES6 environment, you'd be getting a temporal deadzone error, but in Babel-transpiled code, you get undefined.

I agree that it's potentially confusing, but if you want this changed, that should be discussed in the specification repo here: https://github.com/jeffmo/es-class-fields-and-static-properties I have absolutely no control over this. @jeffmo may be able to clarify, but from that repo, it actually seems like Babel 6 sans-decorators is actually what is incorrect here, not this plugin.

@loganfsmyth
Copy link
Owner

I filed a bug to get clarification: tc39/proposal-class-public-fields#40

@uMaxmaxmaximus
Copy link

uMaxmaxmaximus commented Jun 23, 2016

It seems to me improper specification and should not follow it. And I think that in this way many believe. but where to go. I have to write a decorator such as:

@schema({
 foo: User
})
class User {}

but this not works

works just ugly way

class User {}
User.schema = {lol: User}

But what if I want to create a class in expression?

return (function(){
  class User {}
  User.schema = {lol: User}
  return User
})()

It is obvious that this is a bug, and the specification is not true

@jeffmo
Copy link

jeffmo commented Jun 23, 2016

The class properties spec states (Step 24.ii.a) that the initializer scope inherits from the class-body scope; And the ES6 spec states (Step 4) that the class body scope does have access to the class binding (no TDZ). So indeed, the code above should work as expected.

I am planning to write out proper spec text for the next TC39 meeting at the end of July, so hopefully this will reduce confusion around some of these things.

@loganfsmyth
Copy link
Owner

Okay! Given the results in tc39/proposal-class-public-fields#40, I'll change this to work better. I won't be able to get to it right away though.

@uMaxmaxmaximus
Copy link

@loganfsmyth I won't be able to get to it right away though.

Need some help from Russian guys)?

@paul-sachs
Copy link

Any word on this getting resolved? It looks like babel 7 will be using this plugin by default. Would be nice for this behaviour to be fixed.

@langri-sha
Copy link

Static properties seem to be working fine (had an additional glance at the output) when the order of plugins is correct. Is this still an issue or can we safely use this plugin if we're using static class properties?

@jsg2021
Copy link

jsg2021 commented Aug 13, 2017

the example given seems fixed in my code, but if you try to reference your decorated class after it's declaration, you get an error:

function deco () {}

export default @deco class Foo {}

console.log(Foo); // <-- error Foo is not defined!

@jsg2021
Copy link

jsg2021 commented Aug 13, 2017

if you don't export and define at the same line, it's fine :/

@mcav
Copy link

mcav commented Nov 20, 2017

I think this is still a problem even when the order of plugins is correct, at least in some configurations. I don't have a testcase I can share yet, but if I find out the cause/solution I'll follow up here.

@louisscruz
Copy link

louisscruz commented Nov 20, 2017

Just so it's known: I had an application that was ejected from create-react-app and it appears that babel-jest's createTransformer loaded things in the opposite order for some reason. I had to do the following:

const babelJest = require('babel-jest');

module.exports = babelJest.createTransformer({
  plugins: [
    'transform-class-properties',
    'transform-decorators-legacy',
    require.resolve('babel-plugin-inline-react-svg')
  ],
  presets: [require.resolve('babel-preset-react-app')],
  babelrc: false
});

Also, I had to do the backwards ordering in my package.json for some reason.

@mcav
Copy link

mcav commented Nov 21, 2017

(Actually I misspoke. Removing flow-strip-types fixed an unrelated issue, but not this one.)

@madCode
Copy link

madCode commented Sep 17, 2018

I'm also having issues with this when the plugins are in the correct order. My .babelrc looks like this:

{
  "presets": ["es2015", "react", "stage-3"],
  "plugins": ["transform-decorators-legacy", "transform-class-properties", "transform-flow-strip-types"]
}

@javadbat
Copy link

is there any solution?

@madCode
Copy link

madCode commented Sep 18, 2018

@javadbat upgrading jest to jest 23 and then setting up babel 7 resolved this specific issue. (Though I'm dealing with others right now, haha)

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

No branches or pull requests