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

Swap expect(ReactNoop) for expect(Scheduler) #14971

Merged
merged 2 commits into from
Feb 28, 2019

Conversation

acdlite
Copy link
Collaborator

@acdlite acdlite commented Feb 28, 2019

In the previous commits, I upgraded our custom Jest matchers for the noop and test renderers to use Scheduler under the hood.

Now that all these matchers are using Scheduler, we can drop support for passing ReactNoop and test roots and always pass Scheduler directly.

@sizebot
Copy link

sizebot commented Feb 28, 2019

Details of bundled changes.

Comparing: ccb2a8a...476d05f

react-test-renderer

File Filesize Diff Gzip Diff Prev Size Current Size Prev Gzip Current Gzip ENV
react-test-renderer.development.js -3.5% -3.4% 507.38 KB 489.42 KB 107.18 KB 103.51 KB UMD_DEV
react-test-renderer.production.min.js -5.2% -6.1% 66.95 KB 63.5 KB 20.59 KB 19.34 KB UMD_PROD
react-test-renderer.development.js 0.0% 0.0% 484.93 KB 484.93 KB 102.36 KB 102.36 KB NODE_DEV
react-test-renderer.production.min.js 0.0% 0.0% 63.12 KB 63.12 KB 19.07 KB 19.07 KB NODE_PROD
ReactTestRenderer-dev.js 0.0% 0.0% 494.98 KB 494.98 KB 101.97 KB 101.98 KB FB_WWW_DEV
react-test-renderer-shallow.development.js 0.0% 0.0% 37.2 KB 37.2 KB 9.5 KB 9.5 KB UMD_DEV
react-test-renderer-shallow.production.min.js 0.0% 0.0% 11.12 KB 11.12 KB 3.35 KB 3.35 KB UMD_PROD
react-test-renderer-shallow.development.js 0.0% 0.0% 31.5 KB 31.5 KB 8.14 KB 8.14 KB NODE_DEV
react-test-renderer-shallow.production.min.js 0.0% 0.0% 11.77 KB 11.77 KB 3.65 KB 3.65 KB NODE_PROD

react-noop-renderer

File Filesize Diff Gzip Diff Prev Size Current Size Prev Gzip Current Gzip ENV
react-noop-renderer.development.js -40.5% -36.4% 41.41 KB 24.66 KB 9.49 KB 6.04 KB NODE_DEV
react-noop-renderer.production.min.js -26.7% -27.7% 12.4 KB 9.08 KB 4.19 KB 3.03 KB NODE_PROD
react-noop-renderer-persistent.development.js -40.3% -36.3% 41.53 KB 24.78 KB 9.51 KB 6.05 KB NODE_DEV
react-noop-renderer-persistent.production.min.js -26.7% -27.7% 12.42 KB 9.11 KB 4.2 KB 3.03 KB NODE_PROD
react-noop-renderer-server.development.js 0.0% +0.2% 1.83 KB 1.83 KB 875 B 877 B NODE_DEV
react-noop-renderer-server.production.min.js 0.0% 🔺+0.2% 813 B 813 B 490 B 491 B NODE_PROD

jest-react

File Filesize Diff Gzip Diff Prev Size Current Size Prev Gzip Current Gzip ENV
jest-react.development.js -20.5% -11.2% 9.12 KB 7.25 KB 3.01 KB 2.67 KB NODE_DEV
jest-react.production.min.js -21.4% -12.8% 3.65 KB 2.87 KB 1.63 KB 1.42 KB NODE_PROD
JestReact-dev.js -32.1% -20.0% 6.02 KB 4.09 KB 1.8 KB 1.44 KB FB_WWW_DEV
JestReact-prod.js -31.2% -19.5% 5.1 KB 3.51 KB 1.59 KB 1.28 KB FB_WWW_PROD

Generated by 🚫 dangerJS

@acdlite acdlite force-pushed the swapnoopforscheduler branch from 6ce727e to 00b3c37 Compare February 28, 2019 18:33
In the previous commits, I upgraded our custom Jest matchers for the
noop and test renderers to use Scheduler under the hood.

Now that all these matchers are using Scheduler, we can drop
support for passing ReactNoop and test roots and always pass
Scheduler directly.
I also noticed we don't need to regenerator runtime in noop anymore.
@acdlite acdlite force-pushed the swapnoopforscheduler branch from 00b3c37 to 476d05f Compare February 28, 2019 18:51
@acdlite acdlite merged commit 69060e1 into facebook:master Feb 28, 2019
@@ -599,7 +601,7 @@ describe('ReactIncrementalSideEffects', () => {
// render some higher priority work. The middle content will bailout so
// it remains untouched which means that it should reuse it next time.
ReactNoop.render(<Foo text="foo" step={1} />);
expect(ReactNoop).toFlushAndYield(['Foo', 'Content', 'Bar', 'Bar']);
expect(Scheduler).toFlushAndYield(['Foo', 'Content', 'Bar', 'Bar']);
Copy link
Contributor

@NE-SmallTown NE-SmallTown Sep 25, 2019

Choose a reason for hiding this comment

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

This seems very confused to me because shouldComponentUpdate returns false but the renderer(ReactNoop) still chooses to render it(the Content component) which is very different to the DOM renderer, any details would you mind to share?

NMinhNguyen referenced this pull request in enzymejs/react-shallow-renderer Jan 29, 2020
* Swap expect(ReactNoop) for expect(Scheduler)

In the previous commits, I upgraded our custom Jest matchers for the
noop and test renderers to use Scheduler under the hood.

Now that all these matchers are using Scheduler, we can drop
support for passing ReactNoop and test roots and always pass
Scheduler directly.

* Externalize Scheduler in noop and test bundles

I also noticed we don't need to regenerator runtime in noop anymore.
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