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

allow directives and other comments before flow pragma #9891

Merged
merged 1 commit into from Apr 26, 2019

Conversation

tanhauhau
Copy link
Member

handles 3 cases correctly:

  • allow any arbitrary comments before // @flow flow pragma comment
  • allow any arbitrary strings (assuming it's directive) before // @flow flow pragma comment
  • disallow flow syntax even if // @flow flow pragma content is the first comment, but appear in the middle of the code

it is slightly different than the flow logic I described in #9240 (comment), but I think it's fair enough given the differences in the lexer logic between flow and babel, and it's definitely better than the current logic

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

@@ -95,12 +95,26 @@ export default (superClass: Class<Parser>): Class<Parser> =>
return this.getPluginOption("flow", "all") || this.flowPragma === "flow";
}

finishNodeAt<T: N.Node>(
Copy link
Member

Choose a reason for hiding this comment

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

Would we be able to override finishToken to get the behavior you're looking for of bailing after a semi/string token?

Copy link
Member Author

Choose a reason for hiding this comment

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

hmm.. interesting, let me try that. But which means it would have to handle whitespace, slash tokens for comments right?

@tanhauhau
Copy link
Member Author

Interesting the pipeline failed for unknown reasons

@nicolo-ribaudo
Copy link
Member

It's fixed by #9906

@tanhauhau
Copy link
Member Author

Thanks!

@nicolo-ribaudo nicolo-ribaudo merged commit 277a262 into babel:master Apr 26, 2019
@tanhauhau tanhauhau deleted the tanlh/fix/flow-pragma-2 branch April 27, 2019 04:36
@lock lock bot added the outdated A closed issue/PR that is archived due to age. Recommended to make a new issue label Oct 3, 2019
@lock lock bot locked as resolved and limited conversation to collaborators Oct 3, 2019
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
area: flow outdated A closed issue/PR that is archived due to age. Recommended to make a new issue pkg: parser
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants