Page MenuHomePhabricator

Modify native/.flowconfig, so it includes android specific js files.
ClosedPublic

Authored by przemek on Feb 1 2023, 4:38 AM.
Tags
None
Referenced Files
Unknown Object (File)
Thu, Dec 19, 6:08 PM
Unknown Object (File)
Thu, Dec 19, 5:24 PM
Unknown Object (File)
Thu, Dec 19, 10:25 AM
Unknown Object (File)
Tue, Dec 17, 5:45 AM
Unknown Object (File)
Tue, Dec 17, 5:45 AM
Unknown Object (File)
Tue, Dec 17, 5:45 AM
Unknown Object (File)
Tue, Dec 17, 5:39 AM
Unknown Object (File)
Nov 16 2024, 9:34 PM

Details

Summary

I've spotted that I had no errors in newly created file with extension .android.js There were erros obviously, but were only detected when I deleted .android from name. Those changes to .flowconfig should fix it.

Test Plan

Tested that after changes it checks android files.
Checked ios files as well.

Diff Detail

Repository
rCOMM Comm
Lint
Lint Not Applicable
Unit
Tests Not Applicable

Event Timeline

Interesting... I assumed Flow would have issues doing this (and I think it used to), but I was able to run this command without introducing any Flow errors:

cp native/components/clearable-text-input.react.ios.js native/components/clearable-text-input.react.android.js

That confused me, since I was wondering why the React Native repo has two separate .flowconfigs – one for iOS and one for Android.

I did some more investigation and figured out why. When Flow detects variants, it will typecheck each file individually, but when handling imports of that file it will "default" to one of the variants.

So for instance, if you introduce a new required prop on native/components/clearable-text-input.react.ios.js, Flow will not detect an error even though ChatInputBar does not specify this prop. That's because ChatInputBar is typechecking against native/components/clearable-text-input.react.js.

It looks like the preference runs like this: .js > .ios.js > .android.js (maybe determined by order of module.file_ext in .flowconfig)

So doing something like what the React Native repo has with two separate .flowconfigs help catch this sort of type error. But we don't do it currently for typechecking .js vs. .ios.js, so I think we can skip it for now. This diff is a strict improvement either way

native/.flowconfig
2 ↗(On Diff #21745)

Remove this comment

3 ↗(On Diff #21745)

We can remove this empty line as well

This revision is now accepted and ready to land.Feb 1 2023, 10:24 AM