Skip to content

fix flow-double-nullable-type-cant-be-parsed#8115

Merged
existentialism merged 2 commits intobabel:masterfrom
HassanAli321:flow-double-nullable-type-cant-be-parsed
Jun 7, 2018
Merged

fix flow-double-nullable-type-cant-be-parsed#8115
existentialism merged 2 commits intobabel:masterfrom
HassanAli321:flow-double-nullable-type-cant-be-parsed

Conversation

@HassanAli321
Copy link
Copy Markdown
Contributor

@HassanAli321 HassanAli321 commented Jun 4, 2018

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

@babel-bot
Copy link
Copy Markdown
Collaborator

babel-bot commented Jun 4, 2018 •
edited
Loading

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

@nicolo-ribaudo nicolo-ribaudo added PR: Bug Fix 🐛 A type of pull request used for our changelog categories area: flow pkg: parser labels Jun 4, 2018
nicolo-ribaudo
nicolo-ribaudo reviewed Jun 4, 2018
View reviewed changes
Copy link
Copy Markdown
Member

@nicolo-ribaudo nicolo-ribaudo left a comment •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thank you for working on this bug!

Even if this fix is working, I don't think that it is the right approach. It would be awsome if, when parsing a type, the tokenizer didn't poduce a tt.nullishCoaleshing token but rather two tt.question tokens.

I think that it can bee done by checking that this.state.inType is false when parsing ??:

babel/packages/babel-parser/src/tokenizer/index.js

Lines 627 to 631 in b445b79

readToken_question(): void {
// '?'
const next = this.input.charCodeAt(this.state.pos + 1);
const next2 = this.input.charCodeAt(this.state.pos + 2);
if (next === charCodes.questionMark) {

@HassanAli321
Copy link
Copy Markdown
Contributor Author

HassanAli321 commented Jun 5, 2018

Thanks for sharing your point of view yeah you are right that the tokenizer should read two ?? separately i'll try this approach and update the pr :)

@HassanAli321 HassanAli321 force-pushed the flow-double-nullable-type-cant-be-parsed branch from 1e0e010 to ff80386 Compare June 5, 2018 18:34
@nicolo-ribaudo
Copy link
Copy Markdown
Member

nicolo-ribaudo commented Jun 6, 2018

It needs a test 😛

nicolo-ribaudo
nicolo-ribaudo approved these changes Jun 7, 2018
View reviewed changes
Copy link
Copy Markdown
Member

@nicolo-ribaudo nicolo-ribaudo left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thank you!

@existentialism existentialism merged commit bd44902 into babel:master Jun 7, 2018
@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.

Reviewers

@existentialism existentialism existentialism approved these changes

@nicolo-ribaudo nicolo-ribaudo nicolo-ribaudo approved these changes

Assignees

No one assigned

Labels

area: flow outdated A closed issue/PR that is archived due to age. Recommended to make a new issue pkg: parser PR: Bug Fix 🐛 A type of pull request used for our changelog categories

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

Flow "double nullable" type can't be parsed

4 participants

@HassanAli321 @babel-bot @nicolo-ribaudo @existentialism