Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[flow] make Flow suppressions explicit on the error #26487

Merged
merged 1 commit into from
Mar 27, 2023
Merged

[flow] make Flow suppressions explicit on the error #26487

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
6 changes: 3 additions & 3 deletions packages/internal-test-utils/internalAct.js
View file
Open in desktop
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
View file
Open in desktop
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
View file
Open in desktop
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
View file
Open in desktop
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
1 change: 0 additions & 1 deletion packages/react-devtools-shared/src/__tests__/inspectedElement-test.js
View file
Open in desktop
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
1 change: 0 additions & 1 deletion packages/react-devtools-shared/src/__tests__/legacy/inspectElement-test.js
View file
Open in desktop
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
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

And we should probably also start failing lint on unused suppressions because that is unneeded as of #26479


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

const originalConsoleError = console.error;
// $FlowFixMe
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is there a flowconfig to fail on unused suppressions? Would be nice to continuously remove unused suppressions instead of having to remove them manually.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Two things here:

  1. __test__ are just all ignored per flowconfig. Haven't looked into how bad it'd be to change that (we could still do only @flow in some test headers)
  2. I think unused suppressions warn, but it's a bit tricky since we have at least a few suppressions that are only suppressing in specific configurations (e.g. only in yarn flow native).

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

we have at least a few suppressions that are only suppressing in specific configurations

Right. Might have similar issues with eslint-disable directives

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
20 changes: 0 additions & 20 deletions packages/react-devtools-shared/src/__tests__/storeStressTestConcurrent-test.js
View file
Open in desktop
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
View file
Open in desktop
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
4 changes: 2 additions & 2 deletions packages/react-devtools-shared/src/backend/DevToolsComponentStackFrame.js
View file
Open in desktop
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
4 changes: 2 additions & 2 deletions packages/react-devtools-shared/src/backend/DevToolsConsolePatching.js
View file
Open in desktop
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