[declarations] doesn't work with builtins

[LegNeato]

[declarations] works with your own types or types defined in node modules, but does not suppress errors in flow builtins. See #4916 (comment).

Repro in https://github.com/yamov/FlowDeclarations/.

[LegNeato]

Flow appears to parse all builtins into a global module. When using Context.remove_all_errors cx it doesn't do so for that module, and thus Flow throws errors for the builtins at some path like /private/tmp/flow/flowlib_eca9b02/dom.js:823:13. Adding .*/tmp/.* to [declarations] does not silence the errors as flow appears to treat builtins differently.

[wcandillon]

Thanks for filing this issue. I'm super excited about the use of [declarations] but unfortunately, I'm still getting a lot of type errors.

[LegNeato]

I've been debugging this a bit. I'm looking for a simple place to make the fix but right now it looks like the changes may have to be fairly invasive.

[nodkz]

@LegNeato @mroch I still think that [declarations] is the wrong name. It does not explain what it really means. Maybe [supress] is the better candidate?

For now for supressing wrong or stale declarations in packages i need to manage 2 files. Maybe it helps to find easy fix ;) All what is under [supress] became { declare module.exports: any; }:

.flowconfig

- [ignore]
+ [supress]
.*/node_modules/react-native/Libraries/Lists/FlatList.js
.*/node_modules/react-native/Libraries/Lists/VirtualizedList.js
.*/node_modules/react-native/Libraries/Lists/SectionList.js
.*/node_modules/react-native/Libraries/CameraRoll/CameraRoll.js

.*/node_modules/react-native/Libraries/ReactNative/requireNativeComponent.js
.*/node_modules/react-native/Libraries/ReactNative/renderApplication.js
.*/node_modules/react-native/Libraries/Components/Switch/Switch.js
.*/node_modules/react-native/Libraries/Animated/src/nodes/AnimatedValue.js
.*/node_modules/react-native/Libraries/Animated/src/nodes/AnimatedInterpolation.js

./flow-typed/fix-brocken-libdefs.js

- declare module 'FlatList' { declare module.exports: any; }
- declare module 'VirtualizedList' { declare module.exports: any; }
- declare module 'SectionList' { declare module.exports: any; }
- declare module 'CameraRoll' { declare module.exports: any; }

- declare module 'renderApplication' { declare module.exports: any; }
- declare module 'requireNativeComponent' { declare module.exports: any; }
- declare module 'Switch' { declare module.exports: any; }
- declare module './AnimatedValue' { declare module.exports: any; }
- declare module './nodes/AnimatedValue' { declare module.exports: any; }
- declare module '../nodes/AnimatedValue' { declare module.exports: any; }
- declare module './AnimatedInterpolation' { declare module.exports: any; }
- declare module 'AnimatedInterpolation' { declare module.exports: any; }
- declare module './nodes/AnimatedInterpolation' { declare module.exports: any; }

PS. Please avoid confusing [declarations] name in options.

[nodkz]

Small poll for new name:

[declarations] - 👎(current 0.77-0.78)
[silence] - 👍 (prev 0.76)
[suppress] - ❤️
[supress_as_any] - 😄
[untyped] - 😕
[relax] - 🎉

I think there is no problem to give a new name in 0.79 😇

[wcandillon]

I actually like [declarations] 😅 @nodkz some of the names above don't describe what this option actually does. Like [supress_as_any] or [untyped].

On a side note, I'm still not able to use this option correctly with React and React Native, am I missing something or this is a known issue?

[LegNeato]

@wcandillon That's this issue 😄

[wcandillon]

@LegNeato Thanks a lot 🙋🏼‍♂️ Was just double checking 😉Really appreciate your support.

[lll000111]

@wcandillon

I actually like [declarations]
some of the names above don't describe what this option actually does

[declarations] definitely doesn't.

[haggholm]

For the record, suppress is spelled with two ps. Other than that it’s IMO the most descriptive name, but hardly the focus of this issue…

[hjylewis]

Yeah if you want to change the name can you create a different issue, @nodkz?

This issue documents a bug with [declarations] and flow builtins.

[ericketts]

@LegNeato you mention the changes to get this working potentially being quite invasive, would you be willing to share some of what you discovered as to what the issue actually is and what might be needed to overcome it? I'm not terribly well versed in ocaml but would be happy to see if I can help if you'd be willing to get me up to speed on what you know so far.

[LegNeato]

@ericketts Essentially all I know is in #6631 (comment). I haven't had time to dig in more and am almost exclusively using Rust these days so I don't personally have a pressing need (also I am an OCaml n00b). I basically just started at my landed changes and tried to follow the callers to see how builtins were handled. I didn't see an obvious place where builtin errors are handled differently than non-builtin, but they must be because Context.remove_all_errors cx in my diff doesn't clear their errors. My worry is that flow puts builtins as a "fake" top-level object and we'd have to refactor to differentiate between builtin lib errors (like React) and builtin JS errors (like using console wrong).

[jedwards1211]

@nodkz you forgot [STFU] 😄

[STRML]

Fixed in #8105 by my testing. Please try it out.