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

React Native 0.56.rc-5 no longer works without Flow #20030

Closed
LinusU opened this issue Jul 4, 2018 · 18 comments
Closed

React Native 0.56.rc-5 no longer works without Flow #20030

LinusU opened this issue Jul 4, 2018 · 18 comments
Labels
Resolution: Locked This issue was locked by the bot.

Comments

@LinusU
Copy link
Contributor

LinusU commented Jul 4, 2018

Environment

  React Native Environment Info:
    System:
      OS: macOS High Sierra 10.13.5
      CPU: x64 Intel(R) Core(TM) i7-5557U CPU @ 3.10GHz
      Memory: 822.15 MB / 16.00 GB
      Shell: 5.5.1 - /usr/local/bin/zsh
    Binaries:
      Node: 10.5.0 - /usr/local/bin/node
      Yarn: 1.7.0 - /usr/local/bin/yarn
      npm: 6.1.0 - /usr/local/bin/npm
      Watchman: 4.9.0 - /usr/local/bin/watchman
    SDKs:
      iOS SDK:
        Platforms: iOS 11.4, macOS 10.13, tvOS 11.4, watchOS 4.3
      Android SDK:
        Build Tools: 23.0.1, 26.0.3, 27.0.3
        API Levels: 19, 23, 26
    IDEs:
      Xcode: 9.4.1/9F2000 - /usr/bin/xcodebuild
    npmPackages:
      @types/react: ^16.4.6 => 16.4.6 
      @types/react-native: ^0.55.26 => 0.55.26 
      react: 16.4.1 => 16.4.1 
      react-native: 0.56.0 => 0.56.0 
    npmGlobalPackages:
      babel-preset-react-native-typescript: 1.0.0-rc.1
      create-react-native-app: 1.0.0
      react-native-cli: 2.0.1

Description

Starting with version 0.56.0-rc.5, I can no longer compile my app without the Flow plugin included in my babel config. This is unfortunate since I'm using TypeScript instead of Flow, and the babel plugins for TS and Flow cannot be enabled at the same time.

This works in 0.56.0-rc.4 and thus seems to be a regression.

Reproducible Demo

  1. Using version 0.56.0-rc.5 of React Native, switch out the default babel-preset-react-native for babel-preset-react-native-typescript.
  2. Observe the following error:
error: bundling failed: SyntaxError: node_modules/react-native/Libraries/Lists/VirtualizedList.js: Unexpected token (34:12)

  32 | const {computeWindowedRenderLimits} = require('VirtualizeUtils');
  33 | 
> 34 | import type {DangerouslyImpreciseStyleProp} from 'StyleSheet';
     |             ^
  35 | import type {
  36 |   ViewabilityConfig,
  37 |   ViewToken,
    at _class.raise (node_modules/@babel/core/node_modules/babylon/lib/index.js:776:15)
    at _class.unexpected (node_modules/@babel/core/node_modules/babylon/lib/index.js:2079:16)
    at _class.expectContextual (node_modules/@babel/core/node_modules/babylon/lib/index.js:2047:41)
    at _class.parseImport (node_modules/@babel/core/node_modules/babylon/lib/index.js:5205:12)
    at _class.parseImport (node_modules/@babel/core/node_modules/babylon/lib/index.js:9554:48)
    at _class.parseStatementContent (node_modules/@babel/core/node_modules/babylon/lib/index.js:4043:27)
    at _class.parseStatementContent (node_modules/@babel/core/node_modules/babylon/lib/index.js:9605:58)
    at _class.parseStatement (node_modules/@babel/core/node_modules/babylon/lib/index.js:3962:17)
    at _class.parseBlockOrModuleBlockBody (node_modules/@babel/core/node_modules/babylon/lib/index.js:4513:23)
    at _class.parseBlockBody (node_modules/@babel/core/node_modules/babylon/lib/index.js:4500:10)

I haven't dug in too deep yet, but it seems like maybe the files weren't transpiled prior to being published to npm?

@kelset
Copy link
Contributor

kelset commented Jul 4, 2018

Uhm... I'm confused, are you sure that that module works properly?
It seems like you it's 2 hrs old so I don't understand how we could have tested against it :/

Which commit in rc5 do you think could have created the regression compared to rc4?

@yordis
Copy link

yordis commented Jul 4, 2018

@kelset I can ensure you that my app was working before I upgrade to "react-native": "0.56.0", there is some issue with the Babel config.

Worth to mention, I am using create-react-native-app.

@LinusU
Copy link
Contributor Author

LinusU commented Jul 5, 2018

@yordis I think that you want #20015, this issue is when specifically and intentionally removing the transform-flow-strip-types plugin.

@kelset I was actually installing rc.4 instead of rc.5, thinking it was the latest. While still at the rc.4 I configured babel to use TypeScript instead of Flow, and then published that config as babel-preset-react-native-typescript. After that, I discovered that rc.5 was the latest version, so I bumped the version and then started seeing the error.

Upon investigating more, I think that it actually only worked in rc.4 since I was using a small subset of the react native library which happened to not contain any import type statements. With the ongoing transition from PropTypes to Flow, I think that import type became more widespread in the codebase, and thus I hit the error.

It seems like React Native isn't transpiled before publishing to Npm, and thus it's not JavaScript that gets published, it's Flow. Is this intentional?

My goal is to be able to use the Babel 7 to transpile my TypeScript source so that I can write my React Native app in TypeScript. But since Flow and TypeScript cannot be active at the same time in Babel, I cannot have the Flow plugin activated. Currently, I cannot consume the React Native library when not using the Flow plugin since the published files have import type statements (and similar) in them.

@LinusU
Copy link
Contributor Author

LinusU commented Jul 5, 2018

I'm also having the same problem in the final 0.56.0 release.

  React Native Environment Info:
    System:
      OS: macOS High Sierra 10.13.5
      CPU: x64 Intel(R) Core(TM) i7-5557U CPU @ 3.10GHz
      Memory: 822.15 MB / 16.00 GB
      Shell: 5.5.1 - /usr/local/bin/zsh
    Binaries:
      Node: 10.5.0 - /usr/local/bin/node
      Yarn: 1.7.0 - /usr/local/bin/yarn
      npm: 6.1.0 - /usr/local/bin/npm
      Watchman: 4.9.0 - /usr/local/bin/watchman
    SDKs:
      iOS SDK:
        Platforms: iOS 11.4, macOS 10.13, tvOS 11.4, watchOS 4.3
      Android SDK:
        Build Tools: 23.0.1, 26.0.3, 27.0.3
        API Levels: 19, 23, 26
    IDEs:
      Xcode: 9.4.1/9F2000 - /usr/bin/xcodebuild
    npmPackages:
      @types/react: ^16.4.6 => 16.4.6 
      @types/react-native: ^0.55.26 => 0.55.26 
      react: 16.4.1 => 16.4.1 
      react-native: 0.56.0 => 0.56.0 
    npmGlobalPackages:
      babel-preset-react-native-typescript: 1.0.0-rc.1
      create-react-native-app: 1.0.0
      react-native-cli: 2.0.1

@kelset
Copy link
Contributor

kelset commented Jul 5, 2018

👋 Linus!
Thanks for the details, I have to admit I don't know enough about the npm publishing process to give you a proper answer. So I'll ping @hramos && @grabbou whom may answer to that.

That said, I feel that the solution to this may somewhat be related to the other issue you linked (#20015) - we are currently investigating it with the core.

In the meantime, to use Typescript, have you checked if you can use the approach explained here?

@LinusU
Copy link
Contributor Author

LinusU commented Jul 5, 2018

Thanks for pointing me to that TypeScript guide. The drawback of that approach is that it a two-step build time, first compiling the TypeScript with tsc, and then feeding that output into Babel. My goal is to be able to skip tsc altogether and let Babel handle the TypeScript right away.

@kelset
Copy link
Contributor

kelset commented Jul 10, 2018

Hey Linus, thanks for the explanation. Not using TS myself I have to rely on other people in the core / in the FB internal team to understand how doable what you propose is - yesterday I brought this up in a twitter discussion about RN & TS so hopefully now @axe-fb will be able to talk with the metro team and let you know something more.

@LinusU
Copy link
Contributor Author

LinusU commented Jul 10, 2018

Awesome, thank you!

Regarding making TypeScript work out of the box, I actually think that it should be quite easy to add that in a very non-obtrusive way. If we can:

  1. fix .babelrc overrides test not called for every file #20088
  2. only load transform-flow-strip-types when filename doesn't end in .ts(x)
  3. load transform-typescript when filename does end in .ts(x) (this is the default behavior of preset-typescript in recent babel (beta 48 and up I think, RN uses beta 47 at the moment))
  4. add .ts and .tsx to the default list of extensions

Then TypeScript support would just work by renaming files to .ts(x) instead of .js(x), I think that would be amazing 😍

note that it would not require installing TypeScript for people not using it since it would use Babel instead of tsc to transform the TypeScript source into JavaScript code.

@kelset
Copy link
Contributor

kelset commented Jul 10, 2018

Awesome!
I'm cc-ing @ashfurrow since he was the author of the blogpost (and has surely more knowledge than me on the subject)(he's like on holiday atm but should be back in a few days).

(moreover, this could be turned into an RFC as soon as we agree with the core where the new RFC repo will be)

@LinusU
Copy link
Contributor Author

LinusU commented Jul 10, 2018

I'm following react-native-community/releases#38 and are ready to submit an RFC as soon as the process is ironed out ☺️

@ashfurrow
Copy link

Neat! Sounds good – I'm playing catch-up at work but will take a deeper dive into the RFC PR and this issue on the weekend.

@kelset
Copy link
Contributor

kelset commented Jul 26, 2018

👋 folks!
Any update on this? I should have some spare time this coming weekend if we wanna take a stab at it - btw @LinusU the other issue you linked has been closed by the bot.

@LinusU
Copy link
Contributor Author

LinusU commented Jul 26, 2018

Yeah, I saw that. I'll open a new one since it was closed because of the old version problem, this time without the version of the @types package 😅

I'm still very interested in getting this working!

I saw that a newer Babel landed in the metro project. With that babel the TypeScript preset is configured to only consume .ts and .tsx by default. It doesn't seem like the Flow preset is setup to not process those files though, so I need to check if they both can be active at the same time now.

Anyhow, if we just modify the react-native-preset we can easily load typescript for .ts and .tsx, and flow for every other file. That should be very easy.

The only problem is the closed issue, I'm not sure of the cause either. If it has something to do with the internals of Metro, or if it's just that the config for node_modules/react-native is read from somewhere else.

@kelset
Copy link
Contributor

kelset commented Jul 26, 2018

Understood - anyway, can you please ping me via Twitter DMs so we can try to coordinate around this without polluting the issue too much?

@duro
Copy link

duro commented Jul 26, 2018

@kelset @LinusU if you take the approach of using flow on JS and JSX is that going to clash with having a tsconfig with allowJS enabled? I use that mostly to have better support in VSCode, but just throwing that out there for consideration.

@LinusU
Copy link
Contributor Author

LinusU commented Jul 27, 2018

@duro I don't think so since Babel doesn't use the tsconfig at all.

This would just enable Babel to read .ts files, and to strip type information. But it wouldn't actually validate that the TypeScript types are valid. For that the user would have to use e.g. tsc --noEmit.

edit: You would be able to continue to use VS Code with allowJS, since the option to enable Flow is a babel option, which tsc (which VS Code uses behind the scene) doesn't know or care about.

Also, Flow is already enabled in the current React Native babel config, so if it works now, it should continue to work 👍

@LinusU
Copy link
Contributor Author

LinusU commented Aug 6, 2018

So it seems that publishing Flow files to npm instead of JS is intentional. Flow is a requirement in the built system.

The good news is that #20088 is fixed in Babel beta.48, which should ship with the next metro which should be shipped with React Native 0.57 (the first RC should drop today).

The even better news is that native TypeScript support has been merged to Metro as well 🎉
(facebook/metro#209), so in React Native 0.57, there shouldn't be any extra steps for running TypeScript.

With that, I'm going to close this bug ☺️

@vivekvijay

This comment has been minimized.

@facebook facebook locked as resolved and limited conversation to collaborators Aug 6, 2019
@react-native-bot react-native-bot added the Resolution: Locked This issue was locked by the bot. label Aug 6, 2019
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
Resolution: Locked This issue was locked by the bot.
Projects
None yet
Development

No branches or pull requests

8 participants