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.

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: implicit-inexact-object=error #25210

Merged
merged 9 commits into from
Sep 9, 2022

Conversation

kassens
Copy link
Member

@kassens kassens commented Sep 8, 2022

Over the last years, the meaning of the Flow type { a: number } has changed from an inexact object (explicitly written as { a: number, ... }) to an exact object (i.e. {| a: number |}).

This PR makes these implicitly inexact object explicitly inexact by adding the ... and configures flow to make the ambiguous type without either ... or | an error. A following step can then change the default to be exact.

@sizebot
Copy link

sizebot commented Sep 8, 2022

Comparing: 37cc6bf...4b42c10

Critical size changes

Includes critical production bundles, as well as any change greater than 2%:

Name +/- Base Current +/- gzip Base gzip Current gzip
oss-stable/react-dom/cjs/react-dom.production.min.js = 134.97 kB 134.97 kB = 43.23 kB 43.23 kB
oss-experimental/react-dom/cjs/react-dom.production.min.js = 141.69 kB 141.69 kB = 45.23 kB 45.23 kB
facebook-www/ReactDOM-prod.classic.js = 486.24 kB 486.24 kB = 86.55 kB 86.55 kB
facebook-www/ReactDOM-prod.modern.js = 471.52 kB 471.52 kB = 84.32 kB 84.32 kB
facebook-www/ReactDOMForked-prod.classic.js = 486.24 kB 486.24 kB = 86.55 kB 86.55 kB
facebook-relay/flight/ReactFlightNativeRelayClient-dev.js +4.42% 10.96 kB 11.45 kB +1.99% 3.26 kB 3.33 kB
facebook-www/ReactFlightDOMRelayClient-dev.classic.js +3.82% 10.96 kB 11.38 kB +1.44% 3.26 kB 3.31 kB
facebook-www/ReactFlightDOMRelayClient-dev.modern.js +3.82% 10.96 kB 11.38 kB +1.44% 3.26 kB 3.31 kB
oss-experimental/react-client/cjs/react-client-flight.development.js +2.22% 14.19 kB 14.50 kB +0.74% 4.18 kB 4.21 kB
oss-stable-semver/react-client/cjs/react-client-flight.development.js +2.22% 14.19 kB 14.50 kB +0.74% 4.18 kB 4.21 kB
oss-stable/react-client/cjs/react-client-flight.development.js +2.22% 14.19 kB 14.50 kB +0.74% 4.18 kB 4.21 kB

Significant size changes

Includes any change greater than 0.2%:

Expand to show
Name +/- Base Current +/- gzip Base gzip Current gzip
facebook-relay/flight/ReactFlightNativeRelayClient-dev.js +4.42% 10.96 kB 11.45 kB +1.99% 3.26 kB 3.33 kB
facebook-www/ReactFlightDOMRelayClient-dev.classic.js +3.82% 10.96 kB 11.38 kB +1.44% 3.26 kB 3.31 kB
facebook-www/ReactFlightDOMRelayClient-dev.modern.js +3.82% 10.96 kB 11.38 kB +1.44% 3.26 kB 3.31 kB
oss-experimental/react-client/cjs/react-client-flight.development.js +2.22% 14.19 kB 14.50 kB +0.74% 4.18 kB 4.21 kB
oss-stable-semver/react-client/cjs/react-client-flight.development.js +2.22% 14.19 kB 14.50 kB +0.74% 4.18 kB 4.21 kB
oss-stable/react-client/cjs/react-client-flight.development.js +2.22% 14.19 kB 14.50 kB +0.74% 4.18 kB 4.21 kB
oss-experimental/react-server-dom-webpack/cjs/react-server-dom-webpack.development.js +1.73% 18.25 kB 18.56 kB +0.64% 5.15 kB 5.19 kB
oss-stable-semver/react-server-dom-webpack/cjs/react-server-dom-webpack.development.js +1.73% 18.25 kB 18.56 kB +0.64% 5.15 kB 5.19 kB
oss-stable/react-server-dom-webpack/cjs/react-server-dom-webpack.development.js +1.73% 18.25 kB 18.56 kB +0.64% 5.15 kB 5.19 kB
oss-experimental/react-server-dom-webpack/umd/react-server-dom-webpack.development.js +1.66% 19.57 kB 19.89 kB +0.64% 5.28 kB 5.31 kB
oss-stable-semver/react-server-dom-webpack/umd/react-server-dom-webpack.development.js +1.66% 19.57 kB 19.89 kB +0.64% 5.28 kB 5.31 kB
oss-stable/react-server-dom-webpack/umd/react-server-dom-webpack.development.js +1.66% 19.57 kB 19.89 kB +0.64% 5.28 kB 5.31 kB
facebook-relay/flight/ReactFlightNativeRelayServer-dev.js +0.72% 44.56 kB 44.88 kB +0.96% 11.48 kB 11.59 kB
facebook-www/ReactFlightDOMRelayServer-dev.classic.js +0.59% 62.72 kB 63.09 kB +0.81% 16.24 kB 16.37 kB
facebook-www/ReactFlightDOMRelayServer-dev.modern.js +0.59% 62.77 kB 63.14 kB +0.83% 16.26 kB 16.39 kB
oss-stable-semver/react-server/cjs/react-server-flight.development.js +0.26% 45.60 kB 45.71 kB +0.31% 11.69 kB 11.73 kB
oss-stable/react-server/cjs/react-server-flight.development.js +0.26% 45.60 kB 45.71 kB +0.31% 11.69 kB 11.73 kB
oss-experimental/react-server/cjs/react-server-flight.development.js +0.24% 49.34 kB 49.45 kB +0.31% 12.64 kB 12.68 kB

Generated by 🚫 dangerJS against 4b42c10

@sebmarkbage
Copy link
Collaborator

Is there a way to automatically switch to exact ones if it wouldn't cause errors? Most of these should probably be exact.

Copy link
Collaborator

@sebmarkbage sebmarkbage left a comment

Choose a reason for hiding this comment

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

I went through a couple of things and basically they should all be exact. I think we just assumed it was already exact by default.

If we add these ..., then we miss an opportunity to know which ones we've already gone through and explicitly marked.

Can we try making these all exact instead? At least the ones outside devtools?

packages/react-client/src/ReactFlightClient.js Outdated Show resolved Hide resolved
packages/react-client/src/ReactFlightClient.js Outdated Show resolved Hide resolved
packages/react-client/src/ReactFlightClient.js Outdated Show resolved Hide resolved
packages/react-reconciler/src/ReactFiberRoot.new.js Outdated Show resolved Hide resolved
packages/react-server/src/ReactFizzServer.js Outdated Show resolved Hide resolved
packages/react-server/src/ReactFizzServer.js Outdated Show resolved Hide resolved
packages/react-server/src/ReactFizzServer.js Outdated Show resolved Hide resolved
packages/react-server/src/ReactFizzServer.js Outdated Show resolved Hide resolved
Copy link
Member Author

@kassens kassens left a comment

Choose a reason for hiding this comment

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

Defaulted most of them to exact objects now, trying to use my best judgement if that wasn't possible.

@@ -129,17 +129,20 @@ function readRoot<T>(): T {
}

function createPendingChunk(response: Response): PendingChunk {
// $FlowFixMe Flow doesn't support functions as constructors
Copy link
Member Author

Choose a reason for hiding this comment

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

looks like in a current version of Flow support for functions as constructors has gone away: try flow.

In the version here, it returns an inexact object but in the light that it's been already deprecated, I just added the comment that it's no longer supported.

@@ -22,6 +22,7 @@ type HiddenContext = {
// order to reveal the hidden content.
// TODO: Remove `subtreeLanes` context from work loop in favor of this one.
baseLanes: number,
...
Copy link
Member Author

Choose a reason for hiding this comment

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

OffscreenState has an extra property here (cachePool) and needs to be assignable somewhere.

Copy link
Collaborator

Choose a reason for hiding this comment

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

It should probably just add this field:

cachePool: SpawnedCachePool | null,

@@ -839,6 +839,7 @@ function emitModuleChunk(
id: number,
moduleMetaData: ModuleMetaData,
): void {
// $FlowFixMe ModuleMetaData is not a ReactModel
Copy link
Member Author

Choose a reason for hiding this comment

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

Couldn't figure this one out how this is supposed to be allowed…

Copy link
Collaborator

Choose a reason for hiding this comment

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

I can follow up on this one.

@kassens kassens requested a review from sebmarkbage September 8, 2022 21:32
@jackromo888
Copy link

Thanks

@kassens kassens merged commit 8a9e7b6 into facebook:main Sep 9, 2022
@kassens kassens deleted the flow-implicit-inexact-error branch September 9, 2022 14:14
sammy-SC pushed a commit that referenced this pull request Sep 11, 2022
* implicit-inexact-object=error
* default everything ambiguous to exact object
* inexact where exact causes errors
facebook-github-bot pushed a commit to facebook/react-native that referenced this pull request Sep 22, 2022
Summary:
This sync includes the following changes:
- **[0cac4d54c](facebook/react@0cac4d54c )**: Double invoked effects on suspended children ([#25307](facebook/react#25307)) //<Samuel Susla>//
- **[3d615fc14](facebook/react@3d615fc14 )**: Grammar. Removed doubles of the word "the". ([#25295](facebook/react#25295)) //<Victoria Graf>//
- **[6e3bc8a2e](facebook/react@6e3bc8a2e )**: [DevTools] Check if Proxy exists before creating DispatcherProxy ([#25278](facebook/react#25278)) //<Tianyu Yao>//
- **[e7fc04b29](facebook/react@e7fc04b29 )**: [react-dom] Reorganize react-dom internals to match react ([#25277](facebook/react#25277)) //<Josh Story>//
- **[0b54e0047](facebook/react@0b54e0047 )**: Handle rejections to avoid uncaught rejections ([#25272](facebook/react#25272)) //<Sebastian Markbåge>//
- **[c5d06fdc5](facebook/react@c5d06fdc5 )**: [Flight] Fix Webpack Chunk Loading ([#25271](facebook/react#25271)) //<Sebastian Markbåge>//
- **[975b64464](facebook/react@975b64464 )**: [Flight] response.readRoot() -> use(response) ([#25267](facebook/react#25267)) //<Sebastian Markbåge>//
- **[60fbb7b14](facebook/react@60fbb7b14 )**: [Flight] Implement FlightClient in terms of Thenable/Promises instead of throwing Promises ([#25260](facebook/react#25260)) //<Sebastian Markbåge>//
- **[c91a1e03b](facebook/react@c91a1e03b )**: experimental_useEvent ([#25229](facebook/react#25229)) //<Lauren Tan>//
- **[346c7d4c4](facebook/react@346c7d4c4 )**: straightford explicit types ([#25253](facebook/react#25253)) //<Jan Kassens>//
- **[3401e9200](facebook/react@3401e9200 )**: useMemoCache implementation ([#25143](facebook/react#25143)) //<Joseph Savona>//
- **[0556bab32](facebook/react@0556bab32 )**: [Transition Tracing] More Accurate End Time ([#25105](facebook/react#25105)) //<Luna Ruan>//
- **[5fdcd23aa](facebook/react@5fdcd23aa )**: Flow: upgrade to 0.140 ([#25252](facebook/react#25252)) //<Jan Kassens>//
- **[5c43c6f02](facebook/react@5c43c6f02 )**: Unwind the current workInProgress if it's suspended ([#25247](facebook/react#25247)) //<Sebastian Markbåge>//
- **[e52fa4c57](facebook/react@e52fa4c57 )**: Add early exit to strict mode ([#25235](facebook/react#25235)) //<Samuel Susla>//
- **[6aa38e74c](facebook/react@6aa38e74c )**: Flow: enable unsafe-addition error ([#25242](facebook/react#25242)) //<Jan Kassens>//
- **[ba7b6f418](facebook/react@ba7b6f418 )**: Flow: upgrade to 0.132 ([#25244](facebook/react#25244)) //<Jan Kassens>//
- **[9328988c0](facebook/react@9328988c0 )**: Flow: fix Fiber typed as any ([#25241](facebook/react#25241)) //<Jan Kassens>//
- **[c739cef2f](facebook/react@c739cef2f )**: Flow: ReactFiberHotReloading recursive type ([#25225](facebook/react#25225)) //<Jan Kassens>//
- **[c156ecd48](facebook/react@c156ecd48 )**: Add some test coverage for some error cases ([#25240](facebook/react#25240)) //<Sebastian Markbåge>//
- **[3613284dc](facebook/react@3613284dc )**: experimental_use(context) for server components and ssr ([#25226](facebook/react#25226)) //<mofeiZ>//
- **[269c4e975](facebook/react@269c4e975 )**: Prevent infinite re-renders in StrictMode + Offscreen ([#25203](facebook/react#25203)) //<Samuel Susla>//
- **[8003ab9cf](facebook/react@8003ab9cf )**: Flow: remove explicit object syntax ([#25223](facebook/react#25223)) //<Jan Kassens>//
- **[492c6e29e](facebook/react@492c6e29e )**: Flow: upgrade to 0.127 ([#25221](facebook/react#25221)) //<Jan Kassens>//
- **[8a9e7b6ce](facebook/react@8a9e7b6ce )**: Flow: implicit-inexact-object=error ([#25210](facebook/react#25210)) //<Jan Kassens>//
- **[37cc6bf12](facebook/react@37cc6bf12 )**: Remove useDeferredValue and useTransition from Flight subset ([#25215](facebook/react#25215)) //<Sebastian Markbåge>//

Changelog:
[General][Changed] - React Native sync for revisions c28f313...0cac4d5

jest_e2e[run_all_tests]

Reviewed By: rickhanlonii

Differential Revision: D39696377

fbshipit-source-id: 113878d22d6244b8555b5fb86db1da5d43f7cfd9
rickhanlonii pushed a commit that referenced this pull request Oct 5, 2022
* implicit-inexact-object=error
* default everything ambiguous to exact object
* inexact where exact causes errors
rickhanlonii pushed a commit that referenced this pull request Oct 5, 2022
* implicit-inexact-object=error
* default everything ambiguous to exact object
* inexact where exact causes errors
GrinZero added a commit to GrinZero/react that referenced this pull request Nov 7, 2022
* 'main' of ssh://github.com/GrinZero/react: (51 commits)
  Flow: add simple explicit export types to Devtools (facebook#25251)
  [react devtools][easy] Centralize calls to patchConsoleUsingWindowValues (facebook#25222)
  Unwind the current workInProgress if it's suspended (facebook#25247)
  Add early exit to strict mode (facebook#25235)
  fix: prettier ignore removed and fixed (facebook#24811)
  Flow: enable unsafe-addition error (facebook#25242)
  Flow: upgrade to 0.132 (facebook#25244)
  Flow: fix Fiber typed as any (facebook#25241)
  Flow: ReactFiberHotReloading recursive type (facebook#25225)
  Add some test coverage for some error cases (facebook#25240)
  experimental_use(context) for server components and ssr (facebook#25226)
  Flow: upgrade to 0.131 (facebook#25224)
  Prevent infinite re-renders in StrictMode + Offscreen (facebook#25203)
  Flow: remove explicit object syntax (facebook#25223)
  Flow: upgrade to 0.127 (facebook#25221)
  Flow: enable exact_by_default (facebook#25220)
  [react devtools] Don't check for NODE_ENV==='test' because it never is (facebook#25186)
  [react devtools][easy] Change variable names, etc. (facebook#25211)
  Bump async from 2.6.3 to 2.6.4 in /fixtures/concurrent/time-slicing (facebook#24443)
  Flow: implicit-inexact-object=error (facebook#25210)
  ...
OlimpiaZurek pushed a commit to OlimpiaZurek/react-native that referenced this pull request May 22, 2023
Summary:
This sync includes the following changes:
- **[0cac4d54c](facebook/react@0cac4d54c )**: Double invoked effects on suspended children ([facebook#25307](facebook/react#25307)) //<Samuel Susla>//
- **[3d615fc14](facebook/react@3d615fc14 )**: Grammar. Removed doubles of the word "the". ([facebook#25295](facebook/react#25295)) //<Victoria Graf>//
- **[6e3bc8a2e](facebook/react@6e3bc8a2e )**: [DevTools] Check if Proxy exists before creating DispatcherProxy ([facebook#25278](facebook/react#25278)) //<Tianyu Yao>//
- **[e7fc04b29](facebook/react@e7fc04b29 )**: [react-dom] Reorganize react-dom internals to match react ([facebook#25277](facebook/react#25277)) //<Josh Story>//
- **[0b54e0047](facebook/react@0b54e0047 )**: Handle rejections to avoid uncaught rejections ([facebook#25272](facebook/react#25272)) //<Sebastian Markbåge>//
- **[c5d06fdc5](facebook/react@c5d06fdc5 )**: [Flight] Fix Webpack Chunk Loading ([facebook#25271](facebook/react#25271)) //<Sebastian Markbåge>//
- **[975b64464](facebook/react@975b64464 )**: [Flight] response.readRoot() -> use(response) ([facebook#25267](facebook/react#25267)) //<Sebastian Markbåge>//
- **[60fbb7b14](facebook/react@60fbb7b14 )**: [Flight] Implement FlightClient in terms of Thenable/Promises instead of throwing Promises ([facebook#25260](facebook/react#25260)) //<Sebastian Markbåge>//
- **[c91a1e03b](facebook/react@c91a1e03b )**: experimental_useEvent ([facebook#25229](facebook/react#25229)) //<Lauren Tan>//
- **[346c7d4c4](facebook/react@346c7d4c4 )**: straightford explicit types ([facebook#25253](facebook/react#25253)) //<Jan Kassens>//
- **[3401e9200](facebook/react@3401e9200 )**: useMemoCache implementation ([facebook#25143](facebook/react#25143)) //<Joseph Savona>//
- **[0556bab32](facebook/react@0556bab32 )**: [Transition Tracing] More Accurate End Time ([facebook#25105](facebook/react#25105)) //<Luna Ruan>//
- **[5fdcd23aa](facebook/react@5fdcd23aa )**: Flow: upgrade to 0.140 ([facebook#25252](facebook/react#25252)) //<Jan Kassens>//
- **[5c43c6f02](facebook/react@5c43c6f02 )**: Unwind the current workInProgress if it's suspended ([facebook#25247](facebook/react#25247)) //<Sebastian Markbåge>//
- **[e52fa4c57](facebook/react@e52fa4c57 )**: Add early exit to strict mode ([facebook#25235](facebook/react#25235)) //<Samuel Susla>//
- **[6aa38e74c](facebook/react@6aa38e74c )**: Flow: enable unsafe-addition error ([facebook#25242](facebook/react#25242)) //<Jan Kassens>//
- **[ba7b6f418](facebook/react@ba7b6f418 )**: Flow: upgrade to 0.132 ([facebook#25244](facebook/react#25244)) //<Jan Kassens>//
- **[9328988c0](facebook/react@9328988c0 )**: Flow: fix Fiber typed as any ([facebook#25241](facebook/react#25241)) //<Jan Kassens>//
- **[c739cef2f](facebook/react@c739cef2f )**: Flow: ReactFiberHotReloading recursive type ([facebook#25225](facebook/react#25225)) //<Jan Kassens>//
- **[c156ecd48](facebook/react@c156ecd48 )**: Add some test coverage for some error cases ([facebook#25240](facebook/react#25240)) //<Sebastian Markbåge>//
- **[3613284dc](facebook/react@3613284dc )**: experimental_use(context) for server components and ssr ([facebook#25226](facebook/react#25226)) //<mofeiZ>//
- **[269c4e975](facebook/react@269c4e975 )**: Prevent infinite re-renders in StrictMode + Offscreen ([facebook#25203](facebook/react#25203)) //<Samuel Susla>//
- **[8003ab9cf](facebook/react@8003ab9cf )**: Flow: remove explicit object syntax ([facebook#25223](facebook/react#25223)) //<Jan Kassens>//
- **[492c6e29e](facebook/react@492c6e29e )**: Flow: upgrade to 0.127 ([facebook#25221](facebook/react#25221)) //<Jan Kassens>//
- **[8a9e7b6ce](facebook/react@8a9e7b6ce )**: Flow: implicit-inexact-object=error ([facebook#25210](facebook/react#25210)) //<Jan Kassens>//
- **[37cc6bf12](facebook/react@37cc6bf12 )**: Remove useDeferredValue and useTransition from Flight subset ([facebook#25215](facebook/react#25215)) //<Sebastian Markbåge>//

Changelog:
[General][Changed] - React Native sync for revisions c28f313...0cac4d5

jest_e2e[run_all_tests]

Reviewed By: rickhanlonii

Differential Revision: D39696377

fbshipit-source-id: 113878d22d6244b8555b5fb86db1da5d43f7cfd9
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants