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

fix transform-flow-comments for import types #9901

Merged

Conversation

@tanhauhau
Copy link
Member

@tanhauhau tanhauhau commented Apr 26, 2019

Q                       A
Fixed Issues? Fixes #6142
Patch: Bug Fix?
Major: Breaking Change?
Minor: New Feature?
Tests Added + Pass? Yes
Documentation PR Link
Any Dependency Changes?
License MIT
@@ -16,10 +16,12 @@
"@babel/plugin-syntax-flow": "^7.2.0"
},
"peerDependencies": {
"@babel/core": "^7.0.0-0"
"@babel/core": "^7.0.0-0",
"@babel/generator": "^7.0.0-0"

This comment has been minimized.

@nicolo-ribaudo

nicolo-ribaudo Apr 26, 2019
Member

Requiring users to install a new package is a breaking change.
I believe you can import it from @babel/core.

This comment has been minimized.

@tanhauhau

tanhauhau Apr 26, 2019
Author Member

interestingly @babel/core does not export generate function from @babel/generator, should I export it out from @babel/core?

This comment has been minimized.

@nicolo-ribaudo

nicolo-ribaudo Apr 26, 2019
Member

Oh I didn't know that.
If we added a generate export, we couldn't use it in this plugin anyway because it needs to be compatible with @babel/core@^7.0.0. We can add @babel/generator to the dependencies of this plugin.

This comment has been minimized.

@tanhauhau

tanhauhau Apr 27, 2019
Author Member

erm. then is it okay to use @babel/generator as peerDependency? since it's @babel/core's dependency

This comment has been minimized.

@nicolo-ribaudo

nicolo-ribaudo Apr 27, 2019
Member

No, because peer dependencies must be explicitly installed by the user, otherwise npm will log a warning

@existentialism existentialism dismissed their stale review Apr 26, 2019

🤦‍♂️

@tanhauhau tanhauhau force-pushed the tanhauhau:tanlh/fix/flow-comment-import-type branch from 64870e6 to e4dd3b4 May 7, 2019
@babel-bot
Copy link
Collaborator

@babel-bot babel-bot commented May 7, 2019

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

@tanhauhau
Copy link
Member Author

@tanhauhau tanhauhau commented Jul 13, 2019

@existentialism is there something any improvements I can make for this PR to be merged?

Copy link
Member

@existentialism existentialism left a comment

👍

@nicolo-ribaudo nicolo-ribaudo merged commit 8b9af1b into babel:master Jul 13, 2019
5 checks passed
5 checks passed
@babel-bot
babel/repl REPL preview is available
Details
@buildsize
buildsize No significant change
Details
ci/circleci Your tests passed on CircleCI!
Details
@codecov
codecov/project 87.45% (target 80%)
Details
continuous-integration/travis-ci/pr The Travis CI build passed
Details
@lock lock bot added the outdated label Oct 12, 2019
@lock lock bot locked as resolved and limited conversation to collaborators Oct 12, 2019
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Linked issues

Successfully merging this pull request may close these issues.

4 participants