facebook / flow

Adds static typing to JavaScript to improve developer productivity and code quality.
https://flow.org/
MIT License
22.08k stars 1.86k forks source link

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

Closed eapache closed 1 year ago

eapache commented 9 years ago

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 commented 8 years ago

+1

samwgoldman commented 8 years ago

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

reallistic commented 8 years ago

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

ntharim commented 8 years ago

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 commented 8 years ago

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

+1

bgeihsgt commented 8 years ago

I just ran into this as well.

chetstone commented 8 years ago

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 commented 8 years ago

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 commented 8 years ago

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 commented 8 years ago

@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 commented 8 years ago

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 commented 8 years ago

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

DaleLJefferson commented 8 years ago

@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 commented 8 years ago

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

MoOx commented 8 years ago

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 commented 8 years ago

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

MoOx commented 8 years ago

@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 commented 8 years ago

@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 commented 8 years ago

@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 commented 7 years ago

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

kylpo commented 7 years ago

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 commented 7 years ago

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 commented 7 years ago

2015? mm ok.

shettayyy commented 7 years ago

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 commented 7 years ago

I would like to know the reasoning as well

jedrichards commented 7 years ago

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 commented 7 years ago

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 commented 7 years ago

@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 commented 7 years ago

@fagerbua I see.

agrcrobles commented 7 years ago

@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 commented 7 years ago

I was hoping that module.file_ext would allow this to sort-of work, but as @kylpo points out in https://github.com/facebook/flow/issues/945#issuecomment-270304437, 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 commented 6 years ago

any progress on this?

hushicai commented 6 years ago

any progress on this?

Zaporozhec7 commented 5 years ago

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

RobTS commented 5 years ago

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

jamesisaac commented 5 years ago

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: https://github.com/facebook/react-native/issues/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 commented 5 years ago

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 commented 5 years ago

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.

SamChou19815 commented 1 year ago

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