Enable loose mode for class-properties

[rgrochowicz]

Copying the contents of my MobX issue:
After trying out the latest create-react-app and mobx 4's decorate, I found that my classes were no longer observable. At first glance, it looks like it's Babel 7's class property transform.

Here's a codesandbox: https://codesandbox.io/s/623qwprm8k
I'd expect the updated value of FooBabel7 to trigger the autorun.

Differences in babel output:
Source:

class Foo {
  value = 1;
}

Babel 6:

class Foo {
  constructor() {
    this.value = 1;
  }
}

Babel 7:

class Foo {
  constructor() {
    Object.defineProperty(this, "value", {
      configurable: true,
      enumerable: true,
      writable: true,
      value: 1
    });
  }
}

Enabling loose mode reverts the transform back to the Babel 6 behavior.

Simple before/after:
Current behavior:

With loose: true:

Original MobX issue: mobxjs/mobx#1471

[facebook-github-bot]

Thank you for your pull request and welcome to our community. We require contributors to sign our Contributor License Agreement, and we don't seem to have you on file. In order for us to review and merge your code, please sign up at https://code.facebook.com/cla. If you are contributing on behalf of someone else (eg your employer), the individual CLA may not be sufficient and your employer may need the corporate CLA signed.

If you have received this in error or have any questions, please contact us at [email protected]. Thanks!

[gaearon]

My first thought is we want to be closer to the spec, not further from it.
Otherwise your code will rely on something that will eventually break.

[Timer]

I agree with @gaearon here, while it's unfortunate that this is now broken, it resulted from the ecosystem becoming closer in-line with the language specification.

I'm pretty sure our hands are tied here until mobxjs/mobx#1352 is resolved.

Can you point us to the code that broke because of this change? As in what specific file/line(s) that were dependent on the old behavior.

[rgrochowicz]

Ah, running this in a browser with support for class properties (like Chrome Canary) also shows the errant behavior: https://plnkr.co/edit/dXlXUaZ0uDOSk0FUaOqm?p=preview.

Yeah, this is a MobX issue. Sadly, projects using decorate on react-scripts@1 will break when upgrading to react-scripts@2

[rgrochowicz]

I'll go ahead and close this.

Thanks for the quick responses!

💃

[gaearon]

Seems like it would be good to report to MobX that the way they're promoting the usage of decorate() API is not future-friendly.

[gaearon]

I thought about this more and I’m not so confident now.
#4263

[gaearon]

@rgrochowicz Can you please resubmit this PR?

[rgrochowicz]

I've reopened this PR, if that works for you.

[gaearon]

Let's change this to

// Enable loose mode to use assignment instead of defineProperty
// See discussion in https://github.com/facebook/create-react-app/issues/4263

[gaearon]

We also need something to ensure that the transform will be applied even if people target browsers that ship with it (latest Chrome)?

[Timer]

We also need something to ensure that the transform will be applied even if people target browsers that ship with it (latest Chrome)?

Are you referring to a test case?

[gaearon]

Oh I guess it's still explicitly added. Never mind then.