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

Flow comments import export #5675

Merged
merged 2 commits into from May 20, 2017
Merged

Conversation

@lightsofapollo
Copy link
Contributor

lightsofapollo commented Apr 27, 2017

Q A
Patch: Bug Fix? yes
Major: Breaking Change? no
Minor: New Feature? no
Deprecations? no
Spec Compliancy? yes?
Tests Added/Pass? yes
Fixed Tickets Fixes #5538
License MIT
Doc PR
Dependency Changes

While trying to get flow types working in relay-runtime I ran into this issue where babel-plugin-transform-flow-comments is not re-exporting types (export ... from ...).

Includes separate commit /w failing test if this fix is somehow not appropriate.

@mention-bot

This comment has been minimized.

Copy link

mention-bot commented Apr 27, 2017

@lightsofapollo, thanks for your PR! By analyzing the history of the files in this pull request, we identified @hzoo, @danharper and @existentialism to be potential reviewers.

@lightsofapollo lightsofapollo force-pushed the lightsofapollo:flow-comments-import-export branch from f33ec7a to b698139 Apr 27, 2017
@babel-bot

This comment has been minimized.

Copy link
Collaborator

babel-bot commented Apr 27, 2017

Hey @lightsofapollo! It looks like one or more of your builds have failed. I've copied the relevant info below to save you some time.

@codecov

This comment has been minimized.

Copy link

codecov bot commented Apr 27, 2017

Codecov Report

Merging #5675 into 7.0 will increase coverage by 0.02%.
The diff coverage is 100%.

Impacted file tree graph

@@            Coverage Diff            @@
##             7.0    #5675      +/-   ##
=========================================
+ Coverage   84.4%   84.42%   +0.02%     
=========================================
  Files        284      284              
  Lines       9749     9749              
  Branches    2733     2733              
=========================================
+ Hits        8229     8231       +2     
+ Misses      1008     1007       -1     
+ Partials     512      511       -1
Impacted Files Coverage Δ
.../babel-plugin-transform-flow-comments/src/index.js 100% <100%> (+6.89%) ⬆️
packages/babel-helper-call-delegate/src/index.js 64% <0%> (-4%) ⬇️
packages/babel-traverse/src/path/modification.js 73.78% <0%> (-0.98%) ⬇️
...bel-plugin-transform-es2015-classes/src/vanilla.js 90.17% <0%> (ø) ⬆️
packages/babel-traverse/src/path/context.js 86.2% <0%> (+0.86%) ⬆️
packages/babel-traverse/src/visitors.js 86.66% <0%> (+0.95%) ⬆️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 925b644...b698139. Read the comment docs.

@loganfsmyth

This comment has been minimized.

Copy link
Member

loganfsmyth commented Apr 27, 2017

I'm curious, have you explored going with .js.flow files instead? We've actually talked about deprecating this plugin since it seems like an ugly workflow in general.

@lightsofapollo

This comment has been minimized.

Copy link
Contributor Author

lightsofapollo commented Apr 27, 2017

@loganfsmyth For my own general use .flow works much better... In this case I am trying to "teach" relay-runtime (which I am not all that familiar /w) to output stuff which contains flow definitions (either .flow or comments).

graphql-js does this really easily by simply copy/pasting the src/ and changing .js to .js.flow... It works well there because the standard module system is being used (instead of @providesModule)...

This is the result of random debugging and hopefully is useful to someone else (even if I ultimately end up /w .flow implementation of relay-runtime)

@hzoo

This comment has been minimized.

Copy link
Member

hzoo commented Apr 27, 2017

If anything we should move it out of this repo if someone else wants to maintain it since we'd like to deprecate. Other issues like #5600, and issues with classes etc have the similar issues

@hzoo hzoo added the area: flow label Apr 27, 2017
@hzoo

This comment has been minimized.

Copy link
Member

hzoo commented May 20, 2017

Will merge but yeah we need to figure out what to do with the plugin (deprecate?)

@hzoo hzoo merged commit 6928695 into babel:7.0 May 20, 2017
3 checks passed
3 checks passed
ci/circleci Your tests passed on CircleCI!
Details
codecov/project 84.42% (target 80%)
Details
continuous-integration/travis-ci/pr The Travis CI build passed
Details
@lock lock bot added the outdated label Oct 6, 2019
@lock lock bot locked as resolved and limited conversation to collaborators Oct 6, 2019
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
5 participants
You can’t perform that action at this time.