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 doesn't recognize platform-specific react-native files #945

Closed
eapache opened this issue Oct 14, 2015 · 39 comments
Closed

Flow doesn't recognize platform-specific react-native files #945

eapache opened this issue Oct 14, 2015 · 39 comments

Comments

@eapache
Copy link

eapache commented Oct 14, 2015

In recent react-native releases, common practice for files that need platform-specific implementations is to turn foo.js into two files: foo.android.js and foo.ios.js. Flow obviously doesn't know what to make of this, which leads to "Required module not found" errors.

As a temporary workaround you can add module.name_mapper for each such file, but maintaining this list is a lot of work, and the end result is that half your code isn't type-checked at all.

Ideally, flow would recognize this pattern, and be able to type-check both implementations against the import (and against each other) to ensure they implement the same interface.

@rops
Copy link

rops commented Mar 7, 2016

+1

@samwgoldman
Copy link
Member

Yup, this is super valuable. We should have a solution for this.

@reallistic
Copy link

Not sure what the proper way to "upvote" this would be. But:
+1

@ntharim
Copy link

ntharim commented Apr 22, 2016

Would be great to see this being worked on. Ideally this should be configurable to support different platforms like .windows.js or .web.js.

@j-wang
Copy link

j-wang commented May 26, 2016

Just recently started converting a React Native project into flow and encountered this.

+1

@bgeihsgt
Copy link

bgeihsgt commented Jun 9, 2016

I just ran into this as well.

@chetstone
Copy link

Another workaround is to add the following to .flowconfig:

module.file_ext=.ios.js
module.file_ext=.js
module.file_ext=.jsx
module.file_ext=.json

Of course you'll have to edit this if you want to check .android or .web

@MoOx
Copy link

MoOx commented Jun 24, 2016

I think this issue can be closed as module.file_ext can totally handle this without any modification in flow. And it makes things explicit.
Maybe just more docs (maybe another example in the doc)?

@MoOx
Copy link

MoOx commented Jun 24, 2016

I still see an issue here: how can flow validate all files? I mean flow might want to choose .ios.js first, but this might leads to issue if someone update another ext (ex: .web.js, defined after) and do not respect the API of .ios.js version.

@DaleLJefferson
Copy link

DaleLJefferson commented Jun 24, 2016

@MoOx Flow will need to run through all three code paths.

I don't think this works now, but it could work like this

flow check --file_ex=.ios.js,.js
flow check --file_ex=.android.js,.js
flow check --file_ex=.web.js,.js

Maybe you could do it now with 3 different .flowconfig

@bkrem
Copy link

bkrem commented Jul 2, 2016

Thanks for the alternative suggestion @dalejefferson, could you please elaborate on how this works for you? I just get flow: unknown option '--file_ex' when trying this on the CLI.

@MoOx
Copy link

MoOx commented Jul 3, 2016

@bkrem I think there is just a typo, try --file-ext.

@DaleLJefferson
Copy link

@bkrem

I don't think this works now, but it could work like this

It does not work for me, I was just suggesting that it could work like that.

@bkrem
Copy link

bkrem commented Jul 4, 2016

Ah right just a misunderstanding, I thought you meant the suggestion above yours was deprecated rather than your own being a suggestion :)

@MoOx
Copy link

MoOx commented Jul 4, 2016

The problem using the config is that flow will stop on the first file it will find. So if you .ios.js is the first, flow won't check other implementations. That's why trying to do that for each extensions might be a good idea.

@techmentormaria
Copy link

module.file_ext=.ios.js works
but module.file_ext=.android.js doesn't. Anyone else experiencing this?

@MoOx
Copy link

MoOx commented Jul 18, 2016

@mhollweck the problem is that flow will stop for the first file it will find. It won't resolve and try multiple files. So you might need to use flow with multiples config or try using a CLI options to run flow 2 times... (or maybe use ios by default and run 1 additional time with .android.js)...

@techmentormaria
Copy link

@MoOx I know that flow only checks the first file it will find but even if I ONLY check android files it won't recognise my android.js files! Am I the only person with this issue?

@fagerbua
Copy link

@mhollweck : React Native's default .flowconfig is set up to ignore all .android.js files. You can work around this by changing -.*/*[.]android.js to .*/node_modules/.*/*[.]android.js in the .flowconfig of your project.

@jarretmoses
Copy link

Any progress on this? I currently have this issue with react-native 0.35 and flow-bin 0.32

@kylpo
Copy link

kylpo commented Jan 4, 2017

I spent a bit of time seeing if I could get a quick fork with a fix, but I don't understand ocaml.

Seems to me like someone with more experience could undo the lazys in https://github.com/facebook/flow/blob/master/src/services/inference/module_js.ml#L372 to allow the module.file_ext options order to matter. That way, a native project with

module.file_ext=.native.js
module.file_ext=.js

would resolve any index.native.js.flows in a directory before index.js.flow.

This feature is critical for a pattern I'm using to share web and native code as a single module. Really hoping someone will help soon!

@omeid
Copy link

omeid commented Jan 5, 2017

Being able to add filetype/extension specific options should be able to solve this problem.

; Or maybe [options *] ?
[options]
module.global.options=set_here

[options *.js]
module.file_ext=.js

[options *.android.js]
module.file_ext=.android.js

[options *.ios.js]
module.file_ext=.ios.js

[options *.web.js]
module.file_ext=.web.js

Of course, some other variants may work better for configuring the extension type for options, perhaps [options ext.js], or [options extesnion=.ext.js], maybe [options ext=ext.js]?

@yury
Copy link

yury commented Jan 11, 2017

2015? mm ok.

@shettayyy
Copy link

shettayyy commented Apr 24, 2017

Can anyone please update as to why React Native's .flowconfig is configured in a way to ignore .android.js files? I cannot find any info on the same. I have also posted it on stack overflow: Why android files are ignored

@JasonHenson
Copy link

I would like to know the reasoning as well

@jedrichards
Copy link

If I add module.file_ext=.ios.js to my flowconfig then I immediately get an extra 100+ flow errors in my project from within React Native 3rd party code in node_modules ...

@andrewkslv
Copy link

For React Native 0.46.4+ version name your ios components with a regular name *.js and Android components with *.android.js. It should be fine.

@fagerbua
Copy link

@eclipticwld it doesn't seem fine to me. The .flowconfig generated by the React Native CLI still opens as follows:

[ignore]
; We fork some components by platform
.*/*[.]android.js

This will be "fine" only in the sense that Flow will ignore any Android specific files in your project.

@andrewkslv
Copy link

@fagerbua I see.

@agrcrobles
Copy link

@eclipticwld @fagerbua That is not an issue related to flow, that is more like a react-native issue, in order to make flow run through my app on android I avoid running flow in react-native by ignoring react-native in my .flowconfig and importing it as mocked library.

@miracle2k
Copy link

I was hoping that module.file_ext would allow this to sort-of work, but as @kylpo points out in #945 (comment), it seems that the order in which those are given is not respected, which makes interacting between a foo.js and a foo.ios.js file impossible.

If my config is:

module.file_ext=.ios.js
module.file_ext=.js

And I have these files:

audio.ios.js
audio.js

Then importing audio should go to the audio.ios.js file.

@sibelius
Copy link

any progress on this?

1 similar comment
@hushicai
Copy link

any progress on this?

@Zaporozhec7
Copy link

Zaporozhec7 commented Jan 16, 2019

any progress on this? (since more than 3 years issue opened and almost year of last comment)

@RobTS
Copy link

RobTS commented Feb 15, 2019

The workaround works for me, but the dependencies usually show up as "unused import"

@jamesisaac
Copy link
Contributor

jamesisaac commented Mar 9, 2019

For a while React Native itself has been getting around this with 2 separate .flowconfig files, which are run as independent checks:

To me this doesn't seem like an unreasonable approach... the way RN forks off platform-specific versions of modules is essentially a way to create different projects per platform, so it's a fair trade-off that they need to be typechecked independently. I generally use the in-code Platform.OS === 'ios' checks so as not to be going against the JS module system.

Ideally, flow would recognize this pattern, and be able to type-check both implementations against the import (and against each other) to ensure they implement the same interface.

I'm not sure this is really in-line with what RN itself does. From what I see e.g. here: facebook/react-native#22990 it looks like they're now more moving towards Platform.OS checks in-code, and then <Platform><Component>.js files. The only times I'm seeing them still using .ios.js/.android.js is when one of those files returns an UnimplementedView, e.g. https://github.com/facebook/react-native/blob/master/Libraries/Components/CheckBox/CheckBox.ios.js , so the interfaces won't match anyway.

@jamesisaac
Copy link
Contributor

cc @TheSavior Do you have any opinion on what could be best approach for Flow here? And with the new native codegen, is RN moving away from .[ios/android].js files, or are they still going to be a recommended practice?

@jgreen210
Copy link

With react-native, it's theoretically possible to have completely separate sets of JavaScript source-code files for ios and android if you have an index.ios.js and an index.android.js.

https://stackoverflow.com/questions/50272240/does-react-native-perform-tree-shaking is also relevant here.

chrisbobbe added a commit to chrisbobbe/zulip-mobile that referenced this issue May 23, 2020
Part of the RN v0.59.10 -> v0.60.0 upgrade [1]. This commit has no
hard constraints on its position relative to the others in the
series, but at least one change (the `HMRLoadingView.js` ignore) is
not necessary until the upgrade.

Corresponds to facebook/react-native@0f909e331.

Leave in one inexplicable line that reveals huge numbers of errors
if removed, mostly in node_modules/react-native/Libraries (but some
in our code as well, e.g., `deprecated-utility` on the use of
`$Subtype` in `redux-persist/index.js.flow`.

```diff
- module.file_ext=.js
- module.file_ext=.jsx
- module.file_ext=.json
- module.file_ext=.native.js
  module.file_ext=.ios.js // This line
```

It's not there in the RN v0.60.0 template, but it's been in our repo
since e38869a, the upgrade from RN 0.51.0 to 0.52.0.

It doesn't seem to have ever been in the template app. The upgrade
helper showing the `rn-diff-purge` diff from RN 0.51.0 to 0.52.0 [1]
doesn't suggest adding that line, but I don't think that tool was
around when that other upgrade was done. Perhaps it was added from
experimentation, or there was another guide to upgrading.

Anyway, there's an open Flow issue [2], which was open at the time
of e38869a, one resolution of which is to include the line that
was included. So, keep including it.

[1]: https://react-native-community.github.io/upgrade-helper/?from=0.51.0&to=0.52.0

[2]: facebook/flow#945
@SamChou19815
Copy link
Contributor

We now have experimental support for this. https://flow.org/en/docs/react/multiplatform

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

No branches or pull requests