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

Projects
None yet
4 participants
@tanhauhau
Copy link
Contributor

commented Apr 24, 2019

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>(

This comment has been minimized.

Copy link
@loganfsmyth

loganfsmyth Apr 24, 2019

Member

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

This comment has been minimized.

Copy link
@tanhauhau

tanhauhau Apr 24, 2019

Author Contributor

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

@tanhauhau tanhauhau force-pushed the tanhauhau:tanlh/fix/flow-pragma-2 branch from aa3982b to 2f2cc10 Apr 24, 2019

@tanhauhau

This comment has been minimized.

Copy link
Contributor Author

commented Apr 26, 2019

Interesting the pipeline failed for unknown reasons

@nicolo-ribaudo

This comment has been minimized.

Copy link
Member

commented Apr 26, 2019

It's fixed by #9906

@tanhauhau

This comment has been minimized.

Copy link
Contributor Author

commented Apr 26, 2019

Thanks!

@nicolo-ribaudo nicolo-ribaudo merged commit 277a262 into babel:master Apr 26, 2019

0 of 2 checks passed

continuous-integration/travis-ci/pr The Travis CI build could not complete due to an error
Details
ci/circleci Your tests failed on CircleCI
Details

@tanhauhau tanhauhau deleted the tanhauhau:tanlh/fix/flow-pragma-2 branch Apr 27, 2019

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
You can’t perform that action at this time.