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

Read-only AST types #1121

Merged
merged 1 commit into from
Dec 6, 2017
Merged

Read-only AST types #1121

merged 1 commit into from
Dec 6, 2017

Conversation

leebyron
Copy link
Contributor

@leebyron leebyron commented Dec 6, 2017

Potential breaking change: Where previously AST would produce null, it now produces undefined

This tightens up the AST types to produce readonly covariant fields, $ReadOnlyArray in a couple places (not AST yet, perhaps in a future PR), and replaces field?: ?Rule with field?: Rule to exclude the possibility of null from undefined for optional rules, reducing complexity.

**Potential breaking change: Where previously AST would produce `null`, it now produces `undefined`**

This tightens up the AST types to produce readonly covariant fields, $ReadOnlyArray in a couple places (not AST yet, perhaps in a future PR), and replaces `field?: ?Rule` with `field?: Rule` to exclude the possibility of `null` from `undefined` for optional rules, reducing complexity.
@leebyron leebyron mentioned this pull request Dec 6, 2017
@leebyron leebyron merged commit 8e0c599 into master Dec 6, 2017
@leebyron leebyron deleted the readonly-ast branch December 6, 2017 21:48
leebyron added a commit that referenced this pull request Dec 6, 2017
leebyron added a commit that referenced this pull request Dec 6, 2017
This is the second step of #1121 to ensure AST are readonly, marking all arrays as $ReadOnlyArray.
leebyron added a commit that referenced this pull request Dec 6, 2017
This is the second step of #1121 to ensure AST are readonly, marking all arrays as $ReadOnlyArray.
leebyron added a commit that referenced this pull request Dec 7, 2017
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants