Fix default descriptor setting for class properties with decorators

[yhpark]

Q	A
Fixed Issues?	Fixes #7391, Fixes #5519, Fixes loganfsmyth/babel-plugin-transform-decorators-legacy#57, Fixes typeorm/typeorm#1458 and probably more
Patch: Bug Fix?	Yes
Major: Breaking Change?
Minor: New Feature?
Tests Added + Pass?	Yes
Documentation PR	No
Any Dependency Changes?	No
License	MIT

As described in #7391, class properties with decorators are given a descriptor whose writable value is false by default, which created an issue that the value of a class property with decorators is fixed as undefined value when it doesn't have an initializer. This patch changes the behavior to be consistent with the class properties without decorators as handled by proposal-class-properties plugin, where a class property gets a base descriptor with configurable: true, enumerable: true, writable: true (https://github.com/babel/babel/blob/master/packages/babel-plugin-proposal-class-properties/src/index.js#L51-L53).

I also made a small change in applyDecoratedDescriptor helper that determines desc.writable value, since desc.writable is always set to either true or false after the above change is made.

[babel-bot]

Build successful! You can test your changes in the REPL here: https://babeljs.io/repl/build/8268/

[Andarist]

i'd suspect this change touch some output.js test file, but this PR has no such changes, do you know why?

[yhpark]

Sorry for the late reply. There are only 4 fixture-based tests in this package and one of them is affected by this change. I already included the output.js change in this commit (https://github.com/babel/babel/pull/7429/files#diff-798ac9fdf8c8cc407676867ecb8cf1b3). Since there's no plugin depending on this package, no tests outside this package would be affected by this change.

[yhpark]

Can someone please review this PR? This issue makes class property decorators practically unusable.

[jdolle]

I've also run in to this bug and second @yhpark in that it makes class decorators unusable. I'd be willing to help resolve the existing conflicts if someone is interested in reviewing.

[marano]

Please merge this <3 As @jdolle and @yhpark have stated, in the current situation the decorators are unusable in many cases.

[yhpark]

@Andarist (cc. @nicolo-ribaudo)
I have rebased the patch onto the latest master branch. This is a small fix that would affect many people who are trying to use class property decorators. I really hope someone would review and merge this PR. Thanks

[nicolo-ribaudo]

(Probably low priority since we are close to release the new decorators implementation)

[eduard-malakhov]

Hi guys, does anyone know whether there is a way to use property decorators today? As far as I can see, the change proposed in this issue hasn't been integrated into the master. Is a new implementation of decorators available?

[marano]

@eduard-malakhov You need to assign an initial value to the property like this:

@decorator()
property = undefined

[eduard-malakhov]

@marano I've tried assigning a default value, but this still results in a property that is configurable: false which I guess is the reason why the lazyInject decorator from inversify-inject-decorators which I apply fails to redefine the property. However, I am not sure about the root cause, because it is hard to tell at which stage this happens - typescript, babel, or the JS interpreter.

[nicolo-ribaudo]

Merged. Sorry if it took so long; the fix will be released in Babel 7.

[eduard-malakhov]

@nicolo-ribaudo Thank you! I am afraid this patch alone is not enough to make decorators work properly in all cases. I've been playing with this sample project which makes use of inversify-inject-decorators and to make it function properly besides the changes in property descriptor I needed to explicitly delete the property on the object in order to get access to the decorated property from the prototype. You can see this in the repo.

When the object is created, it already contains an own property and therefore the property created by the decorator on the prototype is not accessible unless one deletes the object's property. I am not sure, where this property comes from and whether this is a new behavior because if it has always been this way then it is not clear how inversify-inject-decorators and similar decorators could work in principle.

[nicolo-ribaudo]

The problem is that Babel doesn't allow changing the placement of a field (the decorators you use changes it from being defined on the instance to being defined on the prototype). If anyone wants to open a PR (legacy spec: https://github.com/wycats/javascript-decorators) I will be happy to review it.