Skip to content

[static private] Use explicit descriptors instead of an object#8620

Merged
nicolo-ribaudo merged 2 commits intobabel:masterfrom
nicolo-ribaudo:private-statics-use-descriptor
Sep 5, 2018
Merged

[static private] Use explicit descriptors instead of an object#8620
nicolo-ribaudo merged 2 commits intobabel:masterfrom
nicolo-ribaudo:private-statics-use-descriptor

Conversation

@nicolo-ribaudo
Copy link
Copy Markdown
Member

@nicolo-ribaudo nicolo-ribaudo commented Sep 3, 2018

Q                       A
Fixed Issues?
Patch: Bug Fix?
Major: Breaking Change?
Minor: New Feature?
Tests Added + Pass? Yes
Documentation PR Link
Any Dependency Changes?
License MIT

This is similar to ec69b4b (#8318), which was about private instance fields.

Private properties can be non-writable (thanks to decorators), or have
get/set accessors. If we stored this information on the privateClass
object, we would need to always use Object.getOwnPropertyDescriptor
before reading or writing a property because accessors need to be called
with the correct this context (it should be the actual class, not the
object hat stores the private properties). This commit simplifies that
operation a bit by removing the container object.

It also have another advantage, which instance fields already have
thanks to the use of separate weakmaps: unused private static fields
can be tree-shaken away or garbage-collected, while properties of an
object can't. Also, they can be easilier minified.

This isn't strictly required, but it makes the transform a little bit simpler.

cc @rricard @jridgewell

@babel-bot
Copy link
Copy Markdown
Collaborator

babel-bot commented Sep 3, 2018

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

@nicolo-ribaudo nicolo-ribaudo force-pushed the private-statics-use-descriptor branch from b2736c1 to 0f2ed66 Compare September 3, 2018 19:25
This is similar to ec69b4b, which
was about private instance fields.

Private properties can be non-writable (thanks to decorators), or have
get/set accessors. If we stored this information on the `privateClass`
object, we would need to always use `Object.getOwnPropertyDescriptor`
before reading or writing a property because accessors need to be called
with the correct `this` context (it should be the actual class, not the
object hat stores the private properties). This commit simplifies that
operation a bit by removing the container object.

It also have another advantage, which instance fields already have
thanks to the use of separate weakmaps: unused private static fields
can be tree-shaken away or garbage-collected, while properties of an
object can't. Also, they can be easilier minified.
@nicolo-ribaudo nicolo-ribaudo force-pushed the private-statics-use-descriptor branch from 0f2ed66 to 1f974e9 Compare September 4, 2018 09:18
@nicolo-ribaudo
Copy link
Copy Markdown
Member Author

I'm going to merge this so I can rebase #8130 to check that everything still works.

@nicolo-ribaudo nicolo-ribaudo merged commit c5279ee into babel:master Sep 5, 2018
@nicolo-ribaudo nicolo-ribaudo deleted the private-statics-use-descriptor branch September 5, 2018 13:08
@nicolo-ribaudo nicolo-ribaudo mentioned this pull request Sep 5, 2018
5 tasks
@lock lock bot added the outdated A closed issue/PR that is archived due to age. Recommended to make a new issue label Oct 4, 2019
@lock lock bot locked as resolved and limited conversation to collaborators Oct 4, 2019
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

outdated A closed issue/PR that is archived due to age. Recommended to make a new issue Spec: Class Fields

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants