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

Remove Flow support in React preset #6118

Merged
merged 1 commit into from Aug 21, 2017

Conversation

Projects
None yet
6 participants
@ramasilveyra
Contributor

ramasilveyra commented Aug 16, 2017

Q                       A
Fixed Issues Fixes #6095
Patch: Bug Fix? Y
Major: Breaking Change? Y
Minor: New Feature? N
Tests Added/Pass? Y/Y
Spec Compliancy? N
License MIT
Doc PR
Any Dependency Changes?

Remove Flow support in React preset. Needed to fix an incompatibility issue with typescript preset, another option could be adding a noFlow option.

@xtuc

This comment has been minimized.

Member

xtuc commented Aug 16, 2017

Would be ideal but not a good idea to me because too many users are relying on this behavior.

@chicoxyzzy

This comment has been minimized.

Member

chicoxyzzy commented Aug 16, 2017

Why can't we introduce this breaking change in 7.0?

@xtuc

This comment has been minimized.

Member

xtuc commented Aug 16, 2017

Yes, we could. Let's discuss it at the weekly later today.

@hzoo

hzoo approved these changes Aug 19, 2017

@hzoo

This comment has been minimized.

Member

hzoo commented Aug 19, 2017

@@ -5,7 +5,6 @@
This preset always includes the following plugins:
- [syntax-jsx](https://babeljs.io/docs/plugins/syntax-jsx/)
- [transform-flow-strip-types](https://babeljs.io/docs/plugins/transform-flow-strip-types/)

This comment has been minimized.

@existentialism

existentialism Aug 19, 2017

Member

Wonder if it's worth including a note about how Flow syntax support is no longer enabled in 7.x?

This comment has been minimized.

@ramasilveyra

ramasilveyra Aug 19, 2017

Contributor

Yes, it's worth it :D . Added this:

> Note: Flow syntax support is no longer enabled in v7. For that, you will need to add the [Flow preset](https://babeljs.io/docs/plugins/preset-flow/).
@ramasilveyra

This comment has been minimized.

Contributor

ramasilveyra commented Aug 19, 2017

The Circle CI build started to fail on the yarn step with this error:

error An unexpected error occurred: "EEXIST: file already exists, mkdir '/home/ubuntu/babel/node_modules/babel-plugin-transform-export-extensions'".

And I cannot reproduce on my side, any ideas?

@@ -50,7 +50,7 @@ export default function({ types: t }) {
if (skipStrip) {
throw path.buildCodeFrameError(
"A @flow directive is required when using Flow annotations with " +
"babel-preset-react or the `requireDirective` option.",
"the `requireDirective` option.",

This comment has been minimized.

@ramasilveyra

ramasilveyra Aug 19, 2017

Contributor

Also I just found this, I think that it should be changed in this pr too. If i'm wrong please ping me and I will remove this changes.

@nicolo-ribaudo

This comment has been minimized.

Member

nicolo-ribaudo commented Aug 19, 2017

@ramasilveyra I restarted the Circle CI build, now it passes.

@xtuc

xtuc approved these changes Aug 21, 2017

#6118 (comment) I thought it was against 6.x?

lgtm if it lands in 7 👍

@hzoo

This comment has been minimized.

Member

hzoo commented Aug 21, 2017

@xtuc Was always for 7.0, not planning to do 6.x releases 👍

@hzoo hzoo merged commit 9e4e64d into babel:7.0 Aug 21, 2017

3 checks passed

ci/circleci Your tests passed on CircleCI!
Details
codecov/project 86.08% (target 80%)
Details
continuous-integration/travis-ci/pr The Travis CI build passed
Details

@ramasilveyra ramasilveyra deleted the ramasilveyra:remove-flow branch Aug 21, 2017

@Swapnull Swapnull referenced this pull request Jun 12, 2018

Closed

It doesnt start #5

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment