Skip to content

Commit

Permalink
[flow] make Flow suppressions explicit on the error (#26487)
Browse files Browse the repository at this point in the history
Added an explicit type to all $FlowFixMe suppressions to reduce
over-suppressions of new errors that might be caused on the same lines.

Also removes suppressions that aren't used (e.g. in a `@noflow` file as
they're purely misleading)

Test Plan:
yarn flow-ci
  • Loading branch information
kassens committed Mar 27, 2023
1 parent 768f965 commit afea1d0
Show file tree
Hide file tree
Showing 90 changed files with 164 additions and 209 deletions.
6 changes: 3 additions & 3 deletions packages/internal-test-utils/internalAct.js
Original file line number Diff line number Diff line change
Expand Up @@ -35,7 +35,7 @@ export async function act<T>(scope: () => Thenable<T>): Thenable<T> {
);
}

// $FlowFixMe: Flow doesn't know about global Jest object
// $FlowFixMe[cannot-resolve-name]: Flow doesn't know about global Jest object
if (!jest.isMockFunction(setTimeout)) {
throw Error(
"This version of `act` requires Jest's timer mocks " +
Expand Down Expand Up @@ -70,7 +70,7 @@ export async function act<T>(scope: () => Thenable<T>): Thenable<T> {
// Wait until end of current task/microtask.
await waitForMicrotasks();

// $FlowFixMe: Flow doesn't know about global Jest object
// $FlowFixMe[cannot-resolve-name]: Flow doesn't know about global Jest object
if (jest.isEnvironmentTornDown()) {
error.message =
'The Jest environment was torn down before `act` completed. This ' +
Expand All @@ -79,7 +79,7 @@ export async function act<T>(scope: () => Thenable<T>): Thenable<T> {
}

if (!Scheduler.unstable_hasPendingWork()) {
// $FlowFixMe: Flow doesn't know about global Jest object
// $FlowFixMe[cannot-resolve-name]: Flow doesn't know about global Jest object
jest.runOnlyPendingTimers();
if (Scheduler.unstable_hasPendingWork()) {
// Committing a fallback scheduled additional work. Continue flushing.
Expand Down
10 changes: 5 additions & 5 deletions packages/react-client/src/ReactFlightClient.js
Original file line number Diff line number Diff line change
Expand Up @@ -189,20 +189,20 @@ export function getRoot<T>(response: Response): Thenable<T> {
}

function createPendingChunk<T>(response: Response): PendingChunk<T> {
// $FlowFixMe Flow doesn't support functions as constructors
// $FlowFixMe[invalid-constructor] Flow doesn't support functions as constructors
return new Chunk(PENDING, null, null, response);
}

function createBlockedChunk<T>(response: Response): BlockedChunk<T> {
// $FlowFixMe Flow doesn't support functions as constructors
// $FlowFixMe[invalid-constructor] Flow doesn't support functions as constructors
return new Chunk(BLOCKED, null, null, response);
}

function createErrorChunk<T>(
response: Response,
error: ErrorWithDigest,
): ErroredChunk<T> {
// $FlowFixMe Flow doesn't support functions as constructors
// $FlowFixMe[invalid-constructor] Flow doesn't support functions as constructors
return new Chunk(ERRORED, null, error, response);
}

Expand Down Expand Up @@ -253,15 +253,15 @@ function createResolvedModelChunk<T>(
response: Response,
value: UninitializedModel,
): ResolvedModelChunk<T> {
// $FlowFixMe Flow doesn't support functions as constructors
// $FlowFixMe[invalid-constructor] Flow doesn't support functions as constructors
return new Chunk(RESOLVED_MODEL, value, null, response);
}

function createResolvedModuleChunk<T>(
response: Response,
value: ClientReference<T>,
): ResolvedModuleChunk<T> {
// $FlowFixMe Flow doesn't support functions as constructors
// $FlowFixMe[invalid-constructor] Flow doesn't support functions as constructors
return new Chunk(RESOLVED_MODULE, value, null, response);
}

Expand Down
8 changes: 4 additions & 4 deletions packages/react-client/src/ReactFlightReplyClient.js
Original file line number Diff line number Diff line change
Expand Up @@ -103,7 +103,7 @@ export function processReply(
): ReactJSONValue {
const parent = this;
if (__DEV__) {
// $FlowFixMe
// $FlowFixMe[incompatible-use]
const originalValue = this[key];
if (typeof originalValue === 'object' && originalValue !== value) {
if (objectName(originalValue) !== 'Object') {
Expand Down Expand Up @@ -212,7 +212,7 @@ export function processReply(
}
}

// $FlowFixMe
// $FlowFixMe[incompatible-return]
return value;
}

Expand Down Expand Up @@ -249,13 +249,13 @@ export function processReply(
}

if (typeof value === 'symbol') {
// $FlowFixMe `description` might be undefined
// $FlowFixMe[incompatible-type] `description` might be undefined
const name: string = value.description;
if (Symbol.for(name) !== value) {
throw new Error(
'Only global symbols received from Symbol.for(...) can be passed to Server Functions. ' +
`The symbol Symbol.for(${
// $FlowFixMe `description` might be undefined
// $FlowFixMe[incompatible-type] `description` might be undefined
value.description
}) cannot be found among global symbols.`,
);
Expand Down
6 changes: 3 additions & 3 deletions packages/react-debug-tools/src/ReactDebugHooks.js
Original file line number Diff line number Diff line change
Expand Up @@ -123,7 +123,7 @@ function useState<S>(
hook !== null
? hook.memoizedState
: typeof initialState === 'function'
? // $FlowFixMe: Flow doesn't like mixed types
? // $FlowFixMe[incompatible-use]: Flow doesn't like mixed types
initialState()
: initialState;
hookLog.push({primitive: 'State', stackError: new Error(), value: state});
Expand Down Expand Up @@ -674,15 +674,15 @@ function handleRenderFunctionError(error: any): void {
// that the error is caused by user's code in renderFunction.
// In this case, we should wrap the original error inside a custom error
// so that devtools can give a clear message about it.
// $FlowFixMe: Flow doesn't know about 2nd argument of Error constructor
// $FlowFixMe[extra-arg]: Flow doesn't know about 2nd argument of Error constructor
const wrapperError = new Error('Error rendering inspected component', {
cause: error,
});
// Note: This error name needs to stay in sync with react-devtools-shared
// TODO: refactor this if we ever combine the devtools and debug tools packages
wrapperError.name = 'ReactDebugToolsRenderError';
// this stage-4 proposal is not supported by all environments yet.
// $FlowFixMe Flow doesn't have this type yet.
// $FlowFixMe[prop-missing] Flow doesn't have this type yet.
wrapperError.cause = error;
throw wrapperError;
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -1841,7 +1841,6 @@ describe('InspectedElement', () => {
xyz: 1,
},
});
// $FlowFixMe
const bigInt = BigInt(123); // eslint-disable-line no-undef

await utils.actAsync(() =>
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -873,7 +873,6 @@ describe('InspectedElementContext', () => {
xyz: 1,
},
});
// $FlowFixMe
const bigInt = BigInt(123); // eslint-disable-line no-undef

act(() =>
Expand Down
2 changes: 0 additions & 2 deletions packages/react-devtools-shared/src/__tests__/setupTests.js
Original file line number Diff line number Diff line change
Expand Up @@ -80,7 +80,6 @@ beforeEach(() => {
}

const originalConsoleError = console.error;
// $FlowFixMe
console.error = (...args) => {
const firstArg = args[0];
if (
Expand Down Expand Up @@ -111,7 +110,6 @@ beforeEach(() => {
originalConsoleError.apply(console, args);
};
const originalConsoleWarn = console.warn;
// $FlowFixMe
console.warn = (...args) => {
if (shouldIgnoreConsoleErrorOrWarn(args)) {
// Allows testing how DevTools behaves when it encounters console.warn without cluttering the test output.
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -67,7 +67,6 @@ describe('StoreStressConcurrent', () => {

// 1. Render a normal version of [a, b, c, d, e].
let container = document.createElement('div');
// $FlowFixMe
let root = ReactDOMClient.createRoot(container);
act(() => root.render(<Parent>{[a, b, c, d, e]}</Parent>));
expect(store).toMatchInlineSnapshot(
Expand Down Expand Up @@ -151,7 +150,6 @@ describe('StoreStressConcurrent', () => {
for (let i = 0; i < cases.length; i++) {
// Ensure fresh mount.
container = document.createElement('div');
// $FlowFixMe
root = ReactDOMClient.createRoot(container);

// Verify mounting 'abcde'.
Expand Down Expand Up @@ -181,7 +179,6 @@ describe('StoreStressConcurrent', () => {
// 6. Verify *updates* by reusing the container between iterations.
// There'll be no unmounting until the very end.
container = document.createElement('div');
// $FlowFixMe
root = ReactDOMClient.createRoot(container);
for (let i = 0; i < cases.length; i++) {
// Verify mounting 'abcde'.
Expand Down Expand Up @@ -249,7 +246,6 @@ describe('StoreStressConcurrent', () => {
const snapshots = [];
let container = document.createElement('div');
for (let i = 0; i < steps.length; i++) {
// $FlowFixMe
const root = ReactDOMClient.createRoot(container);
act(() => root.render(<Root>{steps[i]}</Root>));
// We snapshot each step once so it doesn't regress.
Expand Down Expand Up @@ -321,7 +317,6 @@ describe('StoreStressConcurrent', () => {
for (let i = 0; i < steps.length; i++) {
for (let j = 0; j < steps.length; j++) {
container = document.createElement('div');
// $FlowFixMe
const root = ReactDOMClient.createRoot(container);
act(() => root.render(<Root>{steps[i]}</Root>));
expect(print(store)).toMatch(snapshots[i]);
Expand All @@ -338,7 +333,6 @@ describe('StoreStressConcurrent', () => {
for (let i = 0; i < steps.length; i++) {
for (let j = 0; j < steps.length; j++) {
container = document.createElement('div');
// $FlowFixMe
const root = ReactDOMClient.createRoot(container);
act(() =>
root.render(
Expand Down Expand Up @@ -412,7 +406,6 @@ describe('StoreStressConcurrent', () => {
const snapshots = [];
let container = document.createElement('div');
for (let i = 0; i < steps.length; i++) {
// $FlowFixMe
const root = ReactDOMClient.createRoot(container);
act(() =>
root.render(
Expand Down Expand Up @@ -515,7 +508,6 @@ describe('StoreStressConcurrent', () => {

// 2. Verify check Suspense can render same steps as initial fallback content.
for (let i = 0; i < steps.length; i++) {
// $FlowFixMe
const root = ReactDOMClient.createRoot(container);
act(() =>
root.render(
Expand All @@ -540,7 +532,6 @@ describe('StoreStressConcurrent', () => {
for (let j = 0; j < steps.length; j++) {
// Always start with a fresh container and steps[i].
container = document.createElement('div');
// $FlowFixMe
const root = ReactDOMClient.createRoot(container);
act(() =>
root.render(
Expand Down Expand Up @@ -586,7 +577,6 @@ describe('StoreStressConcurrent', () => {
for (let j = 0; j < steps.length; j++) {
// Always start with a fresh container and steps[i].
container = document.createElement('div');
// $FlowFixMe
const root = ReactDOMClient.createRoot(container);
act(() =>
root.render(
Expand Down Expand Up @@ -644,7 +634,6 @@ describe('StoreStressConcurrent', () => {
for (let j = 0; j < steps.length; j++) {
// Always start with a fresh container and steps[i].
container = document.createElement('div');
// $FlowFixMe
const root = ReactDOMClient.createRoot(container);
act(() =>
root.render(
Expand Down Expand Up @@ -694,7 +683,6 @@ describe('StoreStressConcurrent', () => {
for (let j = 0; j < steps.length; j++) {
// Always start with a fresh container and steps[i].
container = document.createElement('div');
// $FlowFixMe
const root = ReactDOMClient.createRoot(container);
act(() =>
root.render(
Expand Down Expand Up @@ -748,7 +736,6 @@ describe('StoreStressConcurrent', () => {
for (let j = 0; j < steps.length; j++) {
// Always start with a fresh container and steps[i].
container = document.createElement('div');
// $FlowFixMe
const root = ReactDOMClient.createRoot(container);
act(() =>
root.render(
Expand Down Expand Up @@ -904,7 +891,6 @@ describe('StoreStressConcurrent', () => {
const snapshots = [];
let container = document.createElement('div');
for (let i = 0; i < steps.length; i++) {
// $FlowFixMe
const root = ReactDOMClient.createRoot(container);
act(() =>
root.render(
Expand All @@ -928,7 +914,6 @@ describe('StoreStressConcurrent', () => {
// which is different from the snapshots above. So we take more snapshots.
const fallbackSnapshots = [];
for (let i = 0; i < steps.length; i++) {
// $FlowFixMe
const root = ReactDOMClient.createRoot(container);
act(() =>
root.render(
Expand Down Expand Up @@ -1062,7 +1047,6 @@ describe('StoreStressConcurrent', () => {
for (let j = 0; j < steps.length; j++) {
// Always start with a fresh container and steps[i].
container = document.createElement('div');
// $FlowFixMe
const root = ReactDOMClient.createRoot(container);
act(() =>
root.render(
Expand Down Expand Up @@ -1114,7 +1098,6 @@ describe('StoreStressConcurrent', () => {
for (let j = 0; j < steps.length; j++) {
// Always start with a fresh container and steps[i].
container = document.createElement('div');
// $FlowFixMe
const root = ReactDOMClient.createRoot(container);
act(() =>
root.render(
Expand Down Expand Up @@ -1181,7 +1164,6 @@ describe('StoreStressConcurrent', () => {
for (let j = 0; j < steps.length; j++) {
// Always start with a fresh container and steps[i].
container = document.createElement('div');
// $FlowFixMe
const root = ReactDOMClient.createRoot(container);
act(() =>
root.render(
Expand Down Expand Up @@ -1233,7 +1215,6 @@ describe('StoreStressConcurrent', () => {
for (let j = 0; j < steps.length; j++) {
// Always start with a fresh container and steps[i].
container = document.createElement('div');
// $FlowFixMe
const root = ReactDOMClient.createRoot(container);
act(() =>
root.render(
Expand Down Expand Up @@ -1285,7 +1266,6 @@ describe('StoreStressConcurrent', () => {
for (let j = 0; j < steps.length; j++) {
// Always start with a fresh container and steps[i].
container = document.createElement('div');
// $FlowFixMe
const root = ReactDOMClient.createRoot(container);
act(() =>
root.render(
Expand Down
3 changes: 0 additions & 3 deletions packages/react-devtools-shared/src/__tests__/utils.js
Original file line number Diff line number Diff line change
Expand Up @@ -46,7 +46,6 @@ export async function actAsync(
const {act: actTestRenderer} = require('react-test-renderer');
const {act: actDOM} = require('react-dom/test-utils');

// $FlowFixMe Flow doesn't know about "await act()" yet
await actDOM(async () => {
await actTestRenderer(async () => {
await cb();
Expand All @@ -55,15 +54,13 @@ export async function actAsync(

if (recursivelyFlush) {
while (jest.getTimerCount() > 0) {
// $FlowFixMe Flow doesn't know about "await act()" yet
await actDOM(async () => {
await actTestRenderer(async () => {
jest.runAllTimers();
});
});
}
} else {
// $FlowFixMe Flow doesn't know about "await act()" yet
await actDOM(async () => {
await actTestRenderer(async () => {
jest.runOnlyPendingTimers();
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -77,7 +77,7 @@ export function describeNativeComponentFrame(
let control;

const previousPrepareStackTrace = Error.prepareStackTrace;
// $FlowFixMe It does accept undefined.
// $FlowFixMe[incompatible-type] It does accept undefined.
Error.prepareStackTrace = undefined;

reentry = true;
Expand All @@ -98,7 +98,7 @@ export function describeNativeComponentFrame(
const Fake = function () {
throw Error();
};
// $FlowFixMe
// $FlowFixMe[prop-missing]
Object.defineProperty(Fake.prototype, 'props', {
set: function () {
// We use a throwing setter instead of frozen or non-writable props
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -45,7 +45,7 @@ export function disableLogs(): void {
value: disabledLog,
writable: true,
};
// $FlowFixMe Flow thinks console is immutable.
// $FlowFixMe[cannot-write] Flow thinks console is immutable.
Object.defineProperties(console, {
info: props,
log: props,
Expand All @@ -69,7 +69,7 @@ export function reenableLogs(): void {
enumerable: true,
writable: true,
};
// $FlowFixMe Flow thinks console is immutable.
// $FlowFixMe[cannot-write] Flow thinks console is immutable.
Object.defineProperties(console, {
log: {...props, value: prevLog},
info: {...props, value: prevInfo},
Expand Down
Loading

0 comments on commit afea1d0

Please sign in to comment.