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

Resolve host configs at build time #12792

Merged
merged 15 commits into from May 19, 2018

Conversation

Projects
None yet
8 participants
@gaearon
Copy link
Member

gaearon commented May 13, 2018

For reviewing, I recommend looking at individual commits (except Prettier ones) but keep in mind some of the shadier things got fixed in final commits (especially in the Flow one). For the "full" view, I recommend looking without whitespace.

  • Our renderers resolve host configs statically
  • Noop renderer tests custom renderer npm infra
  • Source tests pass
  • Bundle tests pass
  • Fix persistent renderer test
  • Use named exports for the renderer
  • Use named exports for host config methods
  • Actually rewrite the reconciler modules to get rid of closures
  • Figure out how to avoid OOM'ing
  • Look into Flow issue in CommitWork
  • Figure out Flow
  • Fix Flow issues in all renderers
  • Misc cleanup
  • Fix hydration Flow issues and get rid of new anys
  • Look again at conditional invariants

@gaearon gaearon force-pushed the gaearon:inline-2 branch 2 times, most recently from b6ea1a9 to 12e0a7a May 13, 2018

@pull-bot

This comment has been minimized.

Copy link

pull-bot commented May 14, 2018

ReactDOM: size: -7.3%, gzip: -6.4%

Details of bundled changes.

Comparing: 96992f2...56ca671

react-dom

File Filesize Diff Gzip Diff Prev Size Current Size Prev Gzip Current Gzip ENV
react-dom.development.js -4.2% -2.8% 644.71 KB 617.62 KB 148.07 KB 143.98 KB UMD_DEV
react-dom.production.min.js -7.3% -6.4% 101.07 KB 93.67 KB 32.37 KB 30.31 KB UMD_PROD
react-dom.development.js -4.3% -2.8% 629.07 KB 601.99 KB 143.99 KB 139.99 KB NODE_DEV
react-dom.production.min.js -7.4% -6.9% 99.48 KB 92.16 KB 31.46 KB 29.29 KB NODE_PROD
react-dom-server.browser.development.js -0.1% -0.2% 101.65 KB 101.51 KB 26.53 KB 26.47 KB UMD_DEV
react-dom-server.browser.development.js -0.2% -0.2% 90.95 KB 90.81 KB 24.27 KB 24.22 KB NODE_DEV
react-dom-server.node.development.js -0.2% -0.2% 92.87 KB 92.73 KB 24.81 KB 24.76 KB NODE_DEV
ReactDOM-dev.js -4.7% -3.0% 654.64 KB 623.72 KB 146.97 KB 142.63 KB FB_WWW_DEV
ReactDOM-prod.js -9.5% -6.3% 301.94 KB 273.21 KB 54.95 KB 51.48 KB FB_WWW_PROD
ReactDOMServer-dev.js -0.4% -0.6% 94.62 KB 94.27 KB 24.1 KB 23.95 KB FB_WWW_DEV

react-art

File Filesize Diff Gzip Diff Prev Size Current Size Prev Gzip Current Gzip ENV
react-art.development.js -9.2% -6.6% 441.23 KB 400.75 KB 96.05 KB 89.74 KB UMD_DEV
react-art.production.min.js -12.1% -11.2% 91.67 KB 80.63 KB 28.01 KB 24.87 KB UMD_PROD
react-art.development.js -11.0% -8.2% 367.06 KB 326.6 KB 77.19 KB 70.82 KB NODE_DEV
react-art.production.min.js -19.9% -19.0% 56.18 KB 44.99 KB 17.27 KB 13.99 KB NODE_PROD
ReactART-dev.js -11.8% -8.8% 375.91 KB 331.57 KB 76.85 KB 70.11 KB FB_WWW_DEV
ReactART-prod.js -20.6% -16.7% 183.96 KB 146.08 KB 30.32 KB 25.26 KB FB_WWW_PROD

react-test-renderer

File Filesize Diff Gzip Diff Prev Size Current Size Prev Gzip Current Gzip ENV
react-test-renderer.development.js -9.5% -6.6% 371.94 KB 336.61 KB 78.06 KB 72.95 KB UMD_DEV
react-test-renderer.production.min.js -18.5% -17.3% 55.91 KB 45.57 KB 17.01 KB 14.07 KB UMD_PROD
react-test-renderer.development.js -9.7% -6.9% 362.76 KB 327.45 KB 75.38 KB 70.2 KB NODE_DEV
react-test-renderer.production.min.js -19.0% -18.6% 55.15 KB 44.67 KB 16.72 KB 13.61 KB NODE_PROD
ReactTestRenderer-dev.js -10.5% -7.2% 371.97 KB 333.1 KB 75.19 KB 69.79 KB FB_WWW_DEV

react-reconciler

File Filesize Diff Gzip Diff Prev Size Current Size Prev Gzip Current Gzip ENV
react-reconciler.development.js -6.4% -3.7% 343.06 KB 321.1 KB 70.78 KB 68.18 KB NODE_DEV
react-reconciler.production.min.js -5.7% -7.1% 47.88 KB 45.16 KB 14.53 KB 13.5 KB NODE_PROD
react-reconciler-persistent.development.js -6.6% -4.1% 342.27 KB 319.69 KB 70.49 KB 67.6 KB NODE_DEV
react-reconciler-persistent.production.min.js -3.5% -5.3% 46.81 KB 45.17 KB 14.27 KB 13.51 KB NODE_PROD

react

File Filesize Diff Gzip Diff Prev Size Current Size Prev Gzip Current Gzip ENV
react.development.js -0.2% -0.4% 57.25 KB 57.11 KB 15.92 KB 15.87 KB UMD_DEV
react.development.js -0.3% -0.4% 47.89 KB 47.75 KB 13.58 KB 13.53 KB NODE_DEV
React-dev.js -0.7% -1.2% 48 KB 47.64 KB 13.19 KB 13.04 KB FB_WWW_DEV

react-native-renderer

File Filesize Diff Gzip Diff Prev Size Current Size Prev Gzip Current Gzip ENV
ReactNativeRenderer-dev.js -6.1% -3.3% 483.96 KB 454.47 KB 102.98 KB 99.59 KB RN_FB_DEV
ReactNativeRenderer-prod.js -12.1% -8.5% 233.76 KB 205.47 KB 39.26 KB 35.92 KB RN_FB_PROD
ReactNativeRenderer-dev.js -6.1% -3.3% 483.62 KB 454.13 KB 102.9 KB 99.52 KB RN_OSS_DEV
ReactNativeRenderer-prod.js -11.9% -8.4% 219.88 KB 193.74 KB 37.04 KB 33.94 KB RN_OSS_PROD
ReactFabric-dev.js -6.1% -3.6% 474.3 KB 445.28 KB 100.94 KB 97.29 KB RN_FB_DEV
ReactFabric-prod.js -12.1% -8.8% 211.62 KB 185.92 KB 35.72 KB 32.57 KB RN_FB_PROD
ReactFabric-dev.js -6.1% -3.6% 474.34 KB 445.32 KB 100.96 KB 97.31 KB RN_OSS_DEV
ReactFabric-prod.js -12.1% -8.8% 211.66 KB 185.96 KB 35.74 KB 32.58 KB RN_OSS_PROD

Generated by 🚫 dangerJS

@gaearon gaearon force-pushed the gaearon:inline-2 branch 10 times, most recently from cf838d4 to 267d1f4 May 14, 2018

gaearon added some commits May 12, 2018

Extract base Jest config
This makes it easier to change the source config without affecting the build test config.
Statically import the host config
This changes react-reconciler to import HostConfig instead of getting it through a function argument.

Rather than start with packages like ReactDOM that want to inline it, I started with React Noop and ensured that *custom* renderers using react-reconciler package still work. To do this, I'm making HostConfig module in the reconciler look at a global variable by default (which, in case of the react-reconciler npm package, ends up being the host config argument in the top-level scope).

This is still very broken.

@gaearon gaearon force-pushed the gaearon:inline-2 branch 6 times, most recently from 34c11a9 to 7c55e9e May 18, 2018

TejasQ added a commit to TejasQ/react that referenced this pull request Aug 26, 2018

Resolve host configs at build time (facebook#12792)
* Extract base Jest config

This makes it easier to change the source config without affecting the build test config.

* Statically import the host config

This changes react-reconciler to import HostConfig instead of getting it through a function argument.

Rather than start with packages like ReactDOM that want to inline it, I started with React Noop and ensured that *custom* renderers using react-reconciler package still work. To do this, I'm making HostConfig module in the reconciler look at a global variable by default (which, in case of the react-reconciler npm package, ends up being the host config argument in the top-level scope).

This is still very broken.

* Add scaffolding for importing an inlined renderer

* Fix the build

* ES exports for renderer methods

* ES modules for host configs

* Remove closures from the reconciler

* Check each renderer's config with Flow

* Fix uncovered Flow issue

We know nextHydratableInstance doesn't get mutated inside this function, but Flow doesn't so it thinks it may be null.
Help Flow.

* Prettier

* Get rid of enable*Reconciler flags

They are not as useful anymore because for almost all cases (except third party renderers) we *know* whether it supports mutation or persistence.

This refactoring means react-reconciler and react-reconciler/persistent third-party packages now ship the same thing.
Not ideal, but this seems worth how simpler the code becomes. We can later look into addressing it by having a single toggle instead.

* Prettier again

* Fix Flow config creation issue

* Fix imprecise Flow typing

* Revert accidental changes
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment