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

await act(async () => ...) #14853

Merged
merged 71 commits into from Apr 2, 2019
Merged

Conversation

threepointone
Copy link
Contributor

@threepointone threepointone commented Feb 14, 2019

TODO -

  • ReactDOM, TestRenderer, NoopRenderer
  • tests for other renderers, I missed them last time around
  • the warnings are placeholders, still thinking about what would be better, open to suggestions
  • implications on concurrent mode

acting

(repasting from the group)

I hacked together an asynchronous version of act(...), and it's kinda nice.

You've seen the synchronous version -

act(() => {
  // updates and stuff
})
// make assertions 

This still works, and gives the same warnings. But if you pass an async function -

await act(async() => {
  // updates and stuff
})
// expect commits and effects to be flushed 
// make assertions 

Neat! I set it up so if you don't await the result from act, a warning gets triggered (with setImmediate) to do so. That makes it a bit harder to have rogue async act() calls in the ether.

You can nest act() calls -

await act(async() => {
  // nest synchronous calls
  act(() => {
    // updates and such    
  })
  // as before, updates and effects are flushed 

  // make assertions 
  await sleep(500) // or for a promise to resolve, or whatever
  // more assertions maybe 

  // nest asynchronous calls too 
  await act(async () => {
    // mutations and such 
    // more awaits 
    // and maybe an event or two
  })
  // more assertions 
})

I implemented a cheap form of unrolling safety, so if a previous act() gets closed before any subsequent act() calls, a warning gets triggered. This should prevent most interleaving attempts, and maintain a tree-like shape of act() blocks.

pros -

  • works with async/await, solves oss problems cleanly
  • the sync api is preserved
  • the warning is preserved
  • works with fake timers/fb

cons -

  • can't guarantee batching after the first await in an act block (this will get better when concurrent?)
  • less restrictive than the sync model, and starts to feel more opt-in than opt-out (eg- someone could just wrap their entire test with an act() call... which might be fine?)
  • exposes a secret api on react dom to implement it, dunno how you feel about that

@sizebot
Copy link

sizebot commented Feb 14, 2019

ReactDOM: size: 0.0%, gzip: 0.0%

Details of bundled changes.

Comparing: 9307932...e49e043

react-dom

File Filesize Diff Gzip Diff Prev Size Current Size Prev Gzip Current Gzip ENV
react-dom.development.js 0.0% +0.1% 815.68 KB 816.07 KB 184.46 KB 184.62 KB UMD_DEV
react-dom.production.min.js 0.0% 0.0% 105.16 KB 105.18 KB 33.98 KB 33.98 KB UMD_PROD
react-dom.profiling.min.js 0.0% +0.1% 108.13 KB 108.16 KB 34.61 KB 34.64 KB UMD_PROFILING
react-dom.development.js 0.0% +0.1% 810.07 KB 810.46 KB 182.86 KB 183.02 KB NODE_DEV
react-dom.production.min.js 0.0% 0.0% 105.14 KB 105.16 KB 33.49 KB 33.49 KB NODE_PROD
react-dom.profiling.min.js 0.0% 0.0% 108.24 KB 108.27 KB 34.1 KB 34.11 KB NODE_PROFILING
ReactDOM-dev.js +0.1% +0.1% 834.92 KB 835.35 KB 184.5 KB 184.66 KB FB_WWW_DEV
ReactDOM-prod.js 0.0% 0.0% 331.73 KB 331.84 KB 61.17 KB 61.19 KB FB_WWW_PROD
ReactDOM-profiling.js 0.0% 0.0% 338.18 KB 338.29 KB 62.52 KB 62.54 KB FB_WWW_PROFILING
react-dom-unstable-fire.development.js 0.0% +0.1% 816.03 KB 816.4 KB 184.61 KB 184.76 KB UMD_DEV
react-dom-unstable-fire.production.min.js 0.0% 0.0% 105.17 KB 105.2 KB 33.99 KB 33.99 KB UMD_PROD
react-dom-unstable-fire.profiling.min.js 0.0% +0.1% 108.15 KB 108.17 KB 34.62 KB 34.64 KB UMD_PROFILING
react-dom-unstable-fire.development.js 0.0% +0.1% 810.42 KB 810.78 KB 183 KB 183.15 KB NODE_DEV
react-dom-unstable-fire.production.min.js 0.0% 0.0% 105.15 KB 105.18 KB 33.5 KB 33.5 KB NODE_PROD
react-dom-unstable-fire.profiling.min.js 0.0% 0.0% 108.25 KB 108.28 KB 34.11 KB 34.12 KB NODE_PROFILING
ReactFire-dev.js 0.0% +0.1% 834.13 KB 834.54 KB 184.47 KB 184.63 KB FB_WWW_DEV
ReactFire-prod.js 0.0% 🔺+0.1% 320.39 KB 320.5 KB 58.8 KB 58.83 KB FB_WWW_PROD
ReactFire-profiling.js 0.0% 0.0% 326.79 KB 326.9 KB 60.17 KB 60.2 KB FB_WWW_PROFILING
react-dom-test-utils.development.js +9.5% +8.5% 47.49 KB 52 KB 12.98 KB 14.08 KB UMD_DEV
react-dom-test-utils.production.min.js 🔺+4.7% 🔺+4.9% 9.95 KB 10.42 KB 3.66 KB 3.83 KB UMD_PROD
react-dom-test-utils.development.js +9.6% +8.6% 47.21 KB 51.72 KB 12.91 KB 14.02 KB NODE_DEV
react-dom-test-utils.production.min.js 🔺+4.9% 🔺+4.7% 9.73 KB 10.2 KB 3.59 KB 3.76 KB NODE_PROD
ReactTestUtils-dev.js +10.6% +9.8% 45.17 KB 49.97 KB 12.18 KB 13.38 KB FB_WWW_DEV
react-dom-unstable-native-dependencies.development.js 0.0% 0.0% 60.74 KB 60.76 KB 15.84 KB 15.85 KB UMD_DEV
react-dom-unstable-native-dependencies.development.js 0.0% 0.0% 60.41 KB 60.43 KB 15.72 KB 15.72 KB NODE_DEV
ReactDOMUnstableNativeDependencies-dev.js 0.0% 0.0% 58.88 KB 58.91 KB 14.89 KB 14.9 KB FB_WWW_DEV
react-dom-server.browser.development.js 0.0% 0.0% 134.78 KB 134.78 KB 35.51 KB 35.51 KB UMD_DEV
react-dom-server.browser.production.min.js 0.0% 0.0% 19.05 KB 19.05 KB 7.18 KB 7.18 KB UMD_PROD
react-dom-server.browser.development.js 0.0% 0.0% 130.91 KB 130.91 KB 34.57 KB 34.57 KB NODE_DEV
react-dom-server.browser.production.min.js 0.0% 0.0% 18.98 KB 18.98 KB 7.17 KB 7.17 KB NODE_PROD
ReactDOMServer-dev.js 0.0% -0.0% 133.22 KB 133.22 KB 34.32 KB 34.32 KB FB_WWW_DEV
ReactDOMServer-prod.js 0.0% -0.0% 46.19 KB 46.19 KB 10.63 KB 10.62 KB FB_WWW_PROD
react-dom-server.node.development.js 0.0% 0.0% 132.86 KB 132.86 KB 35.11 KB 35.12 KB NODE_DEV
react-dom-server.node.production.min.js 0.0% 0.0% 19.84 KB 19.84 KB 7.48 KB 7.48 KB NODE_PROD
react-dom-unstable-fizz.browser.development.js 0.0% +0.1% 3.66 KB 3.66 KB 1.45 KB 1.45 KB UMD_DEV
react-dom-unstable-fizz.node.production.min.js 0.0% -0.1% 1.1 KB 1.1 KB 668 B 667 B NODE_PROD

react-art

File Filesize Diff Gzip Diff Prev Size Current Size Prev Gzip Current Gzip ENV
react-art.development.js +0.1% +0.1% 568.43 KB 568.77 KB 122.57 KB 122.71 KB UMD_DEV
react-art.production.min.js 0.0% 0.0% 96.8 KB 96.83 KB 29.69 KB 29.69 KB UMD_PROD
react-art.development.js +0.1% +0.1% 499.34 KB 499.69 KB 105.21 KB 105.36 KB NODE_DEV
react-art.production.min.js 0.0% 0.0% 61.81 KB 61.83 KB 18.92 KB 18.92 KB NODE_PROD
ReactART-dev.js +0.1% +0.2% 509.24 KB 509.61 KB 104.48 KB 104.66 KB FB_WWW_DEV
ReactART-prod.js 0.0% 0.0% 197.31 KB 197.39 KB 33.38 KB 33.39 KB FB_WWW_PROD

react-native-renderer

File Filesize Diff Gzip Diff Prev Size Current Size Prev Gzip Current Gzip ENV
ReactNativeRenderer-dev.js +0.1% +0.1% 630.49 KB 630.86 KB 134.33 KB 134.49 KB RN_FB_DEV
ReactNativeRenderer-prod.js 0.0% 0.0% 246.06 KB 246.14 KB 43.03 KB 43.05 KB RN_FB_PROD
ReactNativeRenderer-profiling.js 0.0% 0.0% 252.31 KB 252.39 KB 44.38 KB 44.4 KB RN_FB_PROFILING
ReactNativeRenderer-dev.js +0.1% +0.1% 630.4 KB 630.77 KB 134.3 KB 134.46 KB RN_OSS_DEV
ReactNativeRenderer-prod.js 0.0% 0.0% 246.07 KB 246.15 KB 43.03 KB 43.04 KB RN_OSS_PROD
ReactNativeRenderer-profiling.js 0.0% 0.0% 252.33 KB 252.41 KB 44.38 KB 44.39 KB RN_OSS_PROFILING
ReactFabric-dev.js +0.1% +0.1% 619.34 KB 619.71 KB 131.68 KB 131.84 KB RN_FB_DEV
ReactFabric-prod.js 0.0% 0.0% 239.46 KB 239.54 KB 41.77 KB 41.79 KB RN_FB_PROD
ReactFabric-profiling.js 0.0% 0.0% 244.8 KB 244.88 KB 43.09 KB 43.11 KB RN_FB_PROFILING
ReactFabric-dev.js +0.1% +0.1% 619.25 KB 619.62 KB 131.63 KB 131.79 KB RN_OSS_DEV
ReactFabric-prod.js 0.0% 0.0% 239.47 KB 239.55 KB 41.76 KB 41.78 KB RN_OSS_PROD
ReactFabric-profiling.js 0.0% 0.0% 244.81 KB 244.89 KB 43.09 KB 43.1 KB RN_OSS_PROFILING

react-test-renderer

File Filesize Diff Gzip Diff Prev Size Current Size Prev Gzip Current Gzip ENV
react-test-renderer.development.js +0.6% +0.9% 510.07 KB 513.35 KB 107.16 KB 108.08 KB UMD_DEV
react-test-renderer.production.min.js 🔺+0.5% 🔺+0.8% 62.75 KB 63.08 KB 19.12 KB 19.28 KB UMD_PROD
react-test-renderer.development.js +0.6% +0.9% 505.59 KB 508.87 KB 106.01 KB 106.93 KB NODE_DEV
react-test-renderer.production.min.js 🔺+0.6% 🔺+0.8% 62.44 KB 62.78 KB 18.96 KB 19.1 KB NODE_PROD
ReactTestRenderer-dev.js +0.7% +1.0% 516.73 KB 520.35 KB 105.77 KB 106.79 KB FB_WWW_DEV
react-test-renderer-shallow.development.js 0.0% -0.0% 38.98 KB 38.98 KB 9.81 KB 9.81 KB UMD_DEV
react-test-renderer-shallow.production.min.js 0.0% -0.0% 11.42 KB 11.42 KB 3.51 KB 3.5 KB UMD_PROD
react-test-renderer-shallow.development.js 0.0% -0.0% 33.2 KB 33.2 KB 8.42 KB 8.41 KB NODE_DEV

react-noop-renderer

File Filesize Diff Gzip Diff Prev Size Current Size Prev Gzip Current Gzip ENV
react-noop-renderer.development.js +10.3% +10.2% 31.23 KB 34.45 KB 7.64 KB 8.42 KB NODE_DEV
react-noop-renderer.production.min.js 🔺+2.3% 🔺+4.3% 10 KB 10.23 KB 3.22 KB 3.36 KB NODE_PROD
react-noop-renderer-persistent.development.js +10.3% +10.3% 31.34 KB 34.57 KB 7.65 KB 8.44 KB NODE_DEV
react-noop-renderer-persistent.production.min.js 🔺+2.3% 🔺+4.3% 10.03 KB 10.25 KB 3.23 KB 3.37 KB NODE_PROD
react-noop-renderer-server.development.js 0.0% -0.1% 1.83 KB 1.83 KB 877 B 876 B NODE_DEV

react-reconciler

File Filesize Diff Gzip Diff Prev Size Current Size Prev Gzip Current Gzip ENV
react-reconciler.development.js +0.1% +0.1% 499.54 KB 499.95 KB 104.03 KB 104.18 KB NODE_DEV
react-reconciler.production.min.js 🔺+0.1% 🔺+0.1% 62.99 KB 63.05 KB 18.74 KB 18.76 KB NODE_PROD
react-reconciler-persistent.development.js +0.1% +0.1% 497.43 KB 497.84 KB 103.17 KB 103.32 KB NODE_DEV
react-reconciler-persistent.production.min.js 🔺+0.1% 🔺+0.1% 63 KB 63.07 KB 18.75 KB 18.76 KB NODE_PROD
react-reconciler-reflection.development.js 0.0% -0.0% 15.79 KB 15.79 KB 4.91 KB 4.91 KB NODE_DEV

Generated by 🚫 dangerJS

act = ReactTestUtils.act;
});

describe('sync', () => {
Copy link
Contributor Author

Choose a reason for hiding this comment

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

these are the ones moved in from TestUtils-test

);
});
});
describe('async', () => {
Copy link
Contributor Author

Choose a reason for hiding this comment

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

these are newly added

});

await sleep(1000);
expect(console.error).toHaveBeenCalledTimes(1);
Copy link
Contributor Author

Choose a reason for hiding this comment

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

but nuh uh, we warn anyway (todo - match on actual warning message)

@threepointone
Copy link
Contributor Author

Assuming there’s no big problem with the approach, I’m going to close this pr later today and continue working on it in my branch. Please feel free to leave comments!

@threepointone
Copy link
Contributor Author

(nevermind, I'll leave it open)

@@ -517,173 +515,4 @@ describe('ReactTestUtils', () => {
ReactTestUtils.renderIntoDocument(<Component />);
expect(mockArgs.length).toEqual(0);
});

Copy link
Contributor Author

Choose a reason for hiding this comment

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

moved these tests into a separate file for .act tests

@threepointone
Copy link
Contributor Author

spoke with dan, moving the act(...) logic into ReactFiberScheduler

@threepointone
Copy link
Contributor Author

The failing test is odd, investigating.

Sunil Pai added 2 commits February 16, 2019 02:25
it's an odd one, because not only does sync act not flush effects correctly, but the async one does (wut). verified it's fine with the dom version.
This was referenced Mar 10, 2020
acra5y added a commit to acra5y/n-dilation-frontend that referenced this pull request May 7, 2020
* Initialized wasm submodule
* Add npm scripts and webpack config to compile wasm and include to assets
* Update express to fix mime type issue with wasm
* Update react to fix async act (see facebook/react#14853)
* Changed error message component props for less tight coupling
@threepointone threepointone deleted the async-act branch September 10, 2023 15:21
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.

None yet