[declarations] not working as expected #7803

nishant-dani · 2019-06-07T17:55:36Z

Flow version:
"flow-bin": "^0.100.0",

Expected behavior

I have the following declarations tag in .flowconfig
[declarations]
./node_modules/.
./graphql-iso-date/.
I am expecting to see no flow type errors in the following file:
node_modules/graphql-iso-date/dist/time/index.js.flow

Actual behavior

Flow throws a lot of errors in that file, disregarding the declarations tag. I was unable to setup a repro on flow try because there appeared to be no way to setup a .flowconfig file.

Link to Try-Flow or Github repo:

nishant-dani · 2019-06-07T18:23:02Z

I had to add a line feed after
./graphql-iso-date/.

nishant-dani · 2019-06-07T18:42:07Z

The problem is still occuring. The line -feed did not help, it only went away temporarily but the problem has surfaced again.

FezVrasta · 2019-06-15T16:45:24Z

I can confirm [declarations] seems to be completely broken on 0.100.0

mroch · 2019-06-16T16:11:20Z

@FezVrasta does it work on previous versions?

mroch · 2019-06-16T16:13:10Z

wondering if it could be the same as #6717

FezVrasta · 2019-06-16T18:12:21Z

No it's definitely a different issue. I didn't have time to test a previous version, I'll give it a shot tomorrow

FezVrasta · 2019-06-19T11:38:21Z

@mroch please try this repro out:

git clone [email protected]:FezVrasta/flow-issue-repro-7803.git
cd flow-issue-repro-7803
yarn --frozen-lockfile
$(yarn bin)/flow

You should see errors coming from @emotion/core even if the Flow configuration is:

[declarations]
<PROJECT_ROOT>/node_modules/.*

I tried even with Flow 0.90.0 and 0.99.0 and I got the same issue.

Most of the errors are because of #6717, but there are some where a $FlowFixMe comment is absent and still throws error:

Error ┈┈┈┈┈┈┈┈┈┈┈┈┈┈┈┈┈┈┈┈┈┈┈┈┈┈┈┈┈┈┈┈┈┈┈┈┈┈┈┈┈┈┈┈┈┈┈┈┈┈┈┈┈┈┈┈┈┈┈┈┈┈┈┈┈┈┈ node_modules/@emotion/cache/src/index.js:237:7

Cannot call StyleSheet with object literal bound to options because possibly uninitialized variable [1] is incompatible
with HTMLElement [2] in property container.

     node_modules/@emotion/cache/src/index.js
 [1]  67│   let container: HTMLElement
        :
     232│
     233│   const cache: EmotionCache = {
     234│     key,
     235│     sheet: new StyleSheet({
     236│       key,
     237│       container,
     238│       nonce: options.nonce,
     239│       speedy: options.speedy
     240│     }),
     241│     nonce: options.nonce,
     242│     inserted,
     243│     registered: {},

     node_modules/@emotion/sheet/src/index.js
 [2]  45│   container: HTMLElement,

FezVrasta · 2019-08-02T15:11:37Z

@mroch did you ever find time to take a look at my repro?

FezVrasta · 2019-09-25T10:22:40Z

This bug still affects Flow (version 0.108.0).

Does anyone know of any workaround to suppress these errors while maintaining the [declarations] behavior?

Fixes facebook#6631, facebook#6717, facebook#7803. There are two major flaws in the original approach: 1. Silencing errors at the merge site does not work for builtins, as those errors are computed later, and 2. Putting that suppression in an if/else where the else block parsed comment error suppressions can cause [declarations] to actually create *more* errors, which was misleading a lot of people and masked the source of bugs. We now suppress entire files in error collation, at the very end of the error reporting process.

FezVrasta · 2019-09-27T10:52:58Z

@mroch I just realized this issue is a dupe of #6717 as you suggested, I thought it was different because I didn't see $FlowFixMe comments in the Flow output, but actually they are just truncated from the Flow report but are there if you look at the source code that is throwing it.

I guess we can close it and focus on #6717

Fixes facebook#6631, facebook#6717, facebook#7803. There are two major flaws in the original approach: 1. Silencing errors at the merge site does not work for builtins, as those errors are computed later, and 2. Putting that suppression in an if/else where the else block parsed comment error suppressions can cause [declarations] to actually create *more* errors, which was misleading a lot of people and masked the source of bugs. We now suppress entire files in error collation, at the very end of the error reporting process.

…[silence] Summary: # Overview In #4916, LegNeato wrote a feature to block reporting of errors in files matching regex patterns as `[silence]` in `.flowconfig`. This sat for about 9 months as an open PR despite strong public support. Just before merging, this was renamed to `[declarations]`. Why do we need it? As you can read in #4916 comments, and as I wrote in #4916 (comment), nearly every Flow version introduces breaking changes. A library written for a given Flow version is unlikely to work with any other version. This is especially true of very complex libraries like `react-native` that seem to break on a monthly basis. Unfortunately, by default, Flow reports errors *internal to a module*, even if these internal errors don't affect any user code. This means that, if a library uses a new feature that is not in the user's current version of Flow, or uses a feature that has been changes (like object types moving to exact-by-default), internal errors will abound. These errors make it difficult to distinguish actual user type errors, and make it impossible to rely on the `flow` binary's exit code for CI. The current alternatives are `[ignore]`, which causes import errors, and `[untyped]`, which casts to `any`. `[declarations]`has the crucial distinction of *leaving exported typedefs intact while ignoring internal errors*, making it much easier to keep typedefs active while your project and its dependencies ebb and flow across many Flow versions. Unfortunately, #4916 did not actually implement `[silence]` correctly, and doubly unfortunate was the naming pushed through in the final hour. This PR fixes the implementation. Additionally, it renames `[declarations]` back to `[silence]`. This was not taken lightly but has been done due to the overwhelming majority of feedback preferring `[silence]` and the confusion surrounding `[declarations]` as it relates to "declaration files" (i.e. `.js.flow` files) and "type declarations" generally. This becomes even more confusing when comparing to other `.flowconfig` directives such as `[include]`. ## What Was Wrong There are two major flaws in the original approach: 1. Silencing errors at the merge site (`Context.remove_all_errors cx`) does not work for builtins, as those errors are computed later, and 2. Putting that suppression in an if/else where the else block parsed comment error suppressions can cause `[declarations]` to actually create *more* errors, which was misleading a lot of people and masked the source of bugs in this feature. ## Commits Fixes #6631, #6717, #7803. 27fa53f is the first commit, which fixes `[declarations]` as written. We now suppress entire files in error collation, at the very end of the error reporting process. ---- The second commit is a0aa29e. This backs out some of the code in #4916 related to file error suppression in the merge phase. This has the benefit of fixing erroneous ignoring of comment suppressions within those files, which was causing unexpected consequences as listed in 2) above. ---- The last commit is a2e11e1. We rename `[declarations]` to `[silence]` This name has not gone over well: #6631 (comment) and is confusing in multiple places, such as: https://github.com/facebook/flow/blob/dd93de0a3796897fe07cca8a3bdc621c992a9880/website/en/docs/declarations/index.md and https://github.com/facebook/flow/blob/dd93de0a3796897fe07cca8a3bdc621c992a9880/tests/lib_interfaces/.flowconfig#L5-L6 It is difficult to grep for, difficult to distinguish versus type declarations, and is widely used interchangeably with "interfaces". For these reasons and many more, it is best to rename this option now that it is usable. Pinging mroch and mrkev who helped get the original version of this in. Pull Request resolved: #8105 Differential Revision: D17776374 Pulled By: mroch fbshipit-source-id: 1d5670e60a6aa4f636a40b609c7e2def5a88c06c

SamChou19815 · 2023-10-09T16:43:13Z

<PROJECT_ROOT> needs to be included

nishant-dani added bug needs triage labels Jun 7, 2019

nishant-dani closed this as completed Jun 7, 2019

nishant-dani reopened this Jun 7, 2019

goodmind added the declarations Issues with library definitions or .js.flow features label Jun 15, 2019

FezVrasta added a commit to FezVrasta/flow-issue-repro-7803 that referenced this issue Jun 19, 2019

repro facebook/flow#7803

40bb181

FezVrasta added a commit to FezVrasta/flow-issue-repro-7803 that referenced this issue Jun 19, 2019

repro facebook/flow#7803

fdbc54d

goodmind removed the needs triage label Aug 2, 2019

STRML mentioned this issue Sep 27, 2019

Fix [declarations] not suppressing errors correctly & rename to [silence] #8105

Closed

FezVrasta mentioned this issue Sep 27, 2019

trying to silence third-party errors with [declarations] seems to re-raise ignored errors instead #6717

Open

SamChou19815 closed this as completed Oct 9, 2023

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[declarations] not working as expected #7803

[declarations] not working as expected #7803

nishant-dani commented Jun 7, 2019

nishant-dani commented Jun 7, 2019

nishant-dani commented Jun 7, 2019

FezVrasta commented Jun 15, 2019

mroch commented Jun 16, 2019

mroch commented Jun 16, 2019

FezVrasta commented Jun 16, 2019

FezVrasta commented Jun 19, 2019 •

edited

Loading

FezVrasta commented Aug 2, 2019

FezVrasta commented Sep 25, 2019

FezVrasta commented Sep 27, 2019

SamChou19815 commented Oct 9, 2023

[declarations] not working as expected #7803

[declarations] not working as expected #7803

Comments

nishant-dani commented Jun 7, 2019

Expected behavior

Actual behavior

nishant-dani commented Jun 7, 2019

nishant-dani commented Jun 7, 2019

FezVrasta commented Jun 15, 2019

mroch commented Jun 16, 2019

mroch commented Jun 16, 2019

FezVrasta commented Jun 16, 2019

FezVrasta commented Jun 19, 2019 • edited Loading

FezVrasta commented Aug 2, 2019

FezVrasta commented Sep 25, 2019

FezVrasta commented Sep 27, 2019

SamChou19815 commented Oct 9, 2023

FezVrasta commented Jun 19, 2019 •

edited

Loading