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

Internal slot properties #7947

Merged
merged 4 commits into from May 17, 2018
Merged

Conversation

@samwgoldman
Copy link
Contributor

samwgoldman commented May 16, 2018

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

Flow 0.72 added support for so-called "internal slot" properties. Parsing support was added in facebook/flow@732eae5, for reference and more information about the feature itself.

† This is a minor breaking change in the parser itself. Flow also has an indexer property, which looks like { [name: K]: V }, where K can be any type. The name is actually optional, so { [K]: V } is also valid. Currently, { [[foo]]: V } is parsed as an indexer property where K = [foo], i.e., a tuple type of a single value with the type foo. It's very unlikely that a tuple type would be used in this position, and the change can be worked around by adding an explicit name or using a type alias. Because this change only affects Flow users, who are already affected by this breaking change if they update to Flow 0.72, I am hoping that it's OK to push this through without a major version bump.

‡ This is also a breaking change to babel-types, because it adds an additional parameter to the t.objectTypeAnnotation builder function. I added the new parameter before the existing exact parameter, which might cause breaking behavior. I'm happy to move the new parameter to the end of the list, but it seemed messy to me. Let me know what you prefer.

@samwgoldman

This comment has been minimized.

Copy link
Contributor Author

samwgoldman commented May 16, 2018

@babel-bot

This comment has been minimized.

Copy link
Collaborator

babel-bot commented May 16, 2018

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

Copy link
Member

nicolo-ribaudo left a comment

LGTM 👍

@@ -0,0 +1,12 @@
declare class C { static [[foo]]: T

This comment has been minimized.

Copy link
@nicolo-ribaudo

nicolo-ribaudo May 16, 2018

Member

Nit: Can this go on a new line?

This comment has been minimized.

Copy link
@samwgoldman

samwgoldman May 16, 2018

Author Contributor

Sure! The logic around inserting newlines wasn't immediately clear to me so I glossed over that part. Happy to make this change.

@existentialism existentialism merged commit b396cdc into babel:master May 17, 2018
4 checks passed
4 checks passed
babel/repl REPL preview is available
Details
ci/circleci Your tests passed on CircleCI!
Details
codecov/project 80.77% (target 80%)
Details
continuous-integration/travis-ci/pr The Travis CI build passed
Details
@samwgoldman samwgoldman deleted the samwgoldman:internal-slot-properties branch May 17, 2018
@sakai-akinobu sakai-akinobu mentioned this pull request Jul 25, 2018
@lock lock bot added the outdated label Oct 5, 2019
@lock lock bot locked as resolved and limited conversation to collaborators Oct 5, 2019
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
4 participants
You can’t perform that action at this time.