Fixed ReactSharedInternals export in UMD bundle by bvaughn · Pull Request #22117 · facebook/react

bvaughn · 2021-08-17T23:32:21Z

Resolves #22113

Prior to this PR, the react UMD bundle contained two ReactSharedInternals objects. The first ReactSharedInternals object came from here:

react/packages/react/src/ReactSharedInternals.js

Lines 15 to 26 in bd25570

    
           const ReactSharedInternals = { 
        
             ReactCurrentDispatcher, 
        
             ReactCurrentBatchConfig, 
        
             ReactCurrentOwner, 
        
             // Used by renderers to avoid bundling object-assign twice in UMD bundles: 
        
             assign, 
        
           }; 
        
           if (__DEV__) { 
        
             ReactSharedInternals.ReactDebugCurrentFrame = ReactDebugCurrentFrame; 
        
             ReactSharedInternals.ReactCurrentActQueue = ReactCurrentActQueue; 
        
           }

The second ReactSharedInternals object (ReactSharedInternals$1 in the bundle) came from here:

react/packages/react/src/forks/ReactSharedInternals.umd.js

Lines 15 to 28 in bd25570

    
           const ReactSharedInternals = { 
        
             ReactCurrentDispatcher, 
        
             ReactCurrentOwner, 
        
             ReactCurrentBatchConfig, 
        
             // Used by renderers to avoid bundling object-assign twice in UMD bundles: 
        
             assign, 
        
             // Re-export the schedule API(s) for UMD bundles. 
        
             // This avoids introducing a dependency on a new UMD global in a minor update, 
        
             // Since that would be a breaking change (e.g. for all existing CodeSandboxes). 
        
             // This re-export is only required for UMD bundles; 
        
             // CJS bundles use the shared NPM package. 
        
             Scheduler, 
        
           };

The second one is the one that ended up being shared via __SECRET_INTERNALS_DO_NOT_USE_OR_YOU_WILL_BE_FIRED. It had a Scheduler key but not a ReactCurrentActQueue key. The fix was to make sure both keys got added to the UMD override.

Testing

Re-built the bundle:

node scripts/rollup/build.js --type=UMD_DEV -- react/index,react-dom/index

Then inspected the output at:

build/node_modules/react/umd/react.development.js
build/node_modules/react-dom/umd/react-dom.development.js

Also dropped both into a small repro app:

<!DOCTYPE html>
<html>
  <head>
    <script src="https://unpkg.com/react@alpha/umd/react.development.js"></script>
    <script src="https://unpkg.com/react-dom@alpha/umd/react-dom.development.js"></script>
  </head>
  <body>
    <script type="text/javascript">
      const container = document.createElement('div');

      document.body.appendChild(container);

      const root = ReactDOM.createRoot(container);
      root.render(React.createElement('div', null, 'Testing...'));
    </script>
  </body>
</html>

sizebot · 2021-08-17T23:36:29Z

Comparing: bd25570...130b59e

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	=	127.58 kB	127.58 kB	=	40.72 kB	40.72 kB
oss-experimental/react-dom/cjs/react-dom.production.min.js	=	130.40 kB	130.40 kB	=	41.65 kB	41.65 kB
facebook-www/ReactDOM-prod.classic.js	=	405.16 kB	405.16 kB	=	75.04 kB	75.04 kB
facebook-www/ReactDOM-prod.modern.js	=	393.72 kB	393.72 kB	=	73.32 kB	73.32 kB
facebook-www/ReactDOMForked-prod.classic.js	=	405.16 kB	405.16 kB	=	75.05 kB	75.05 kB

Significant size changes

Includes any change greater than 0.2%:

(No significant changes)

Generated by 🚫 dangerJS against 130b59e

rickhanlonii · 2021-08-18T02:33:01Z

Is it conceptually possible to flow type this so we catch it if it happens again?

bvaughn · 2021-08-18T05:40:41Z

Is it conceptually possible to flow type this so we catch it if it happens again?

Unclear, since (in this case) the missing properties weren't part of the statically declared type, but rather, were dynamic properties added in the __DEV__ mode build.

I'm going to merge this fix now (since the UMD bundle is broken) but we can follow up on this.

Summary: Post: https://fb.workplace.com/groups/rnsyncsquad/permalink/879923262900946/ This sync includes the following changes: - **[fc3b6a4](facebook/react@fc3b6a411 )**: Fix a few typos ([#22154](facebook/react#22154)) //<Bowen>// - **[986d0e6](facebook/react@986d0e61d )**: [Scheduler] Add tests for isInputPending ([#22140](facebook/react#22140)) //<Andrew Clark>// - **[d54be90](facebook/react@d54be90be )**: Set up test infra for dynamic Scheduler flags ([#22139](facebook/react#22139)) //<Andrew Clark>// - **[7ed0706](facebook/react@7ed0706d7 )**: Remove the warning for setState on unmounted components ([#22114](facebook/react#22114)) //<Dan Abramov>// - **[9eb2aaa](facebook/react@9eb2aaaf8 )**: Fixed ReactSharedInternals export in UMD bundle ([#22117](facebook/react#22117)) //<Brian Vaughn>// - **[bd25570](facebook/react@bd255700d )**: Show a soft error when a text string or number is supplied as a child to non text wrappers ([#22109](facebook/react#22109)) //<Sota>// Changelog: [General][Changed] - React Native sync for revisions 424fe58...bd5bf55 jest_e2e[run_all_tests] Reviewed By: yungsters Differential Revision: D30485521 fbshipit-source-id: c5b92356e9e666eae94536ed31b8de43536419f8

Fixed ReactSharedInternals export in UMD bundle

130b59e

bvaughn requested review from acdlite, gaearon and rickhanlonii August 17, 2021 23:32

facebook-github-bot added CLA Signed React Core Team Opened by a member of the React Core Team labels Aug 17, 2021

gaearon approved these changes Aug 18, 2021

View reviewed changes

rickhanlonii approved these changes Aug 18, 2021

View reviewed changes

bvaughn merged commit 9eb2aaa into facebook:main Aug 18, 2021

bvaughn deleted the fix-umd-build branch August 18, 2021 05:40

zhengjitf pushed a commit to zhengjitf/react that referenced this pull request Apr 15, 2022

Fixed ReactSharedInternals export in UMD bundle (facebook#22117)

c86c894

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Fixed ReactSharedInternals export in UMD bundle#22117

Fixed ReactSharedInternals export in UMD bundle#22117
bvaughn merged 1 commit intofacebook:mainfrom
bvaughn:fix-umd-build

bvaughn commented Aug 17, 2021 •

edited

Loading

Uh oh!

sizebot commented Aug 17, 2021

Uh oh!

rickhanlonii commented Aug 18, 2021

Uh oh!

bvaughn commented Aug 18, 2021 •

edited

Loading

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

5 participants

	const ReactSharedInternals = {
	ReactCurrentDispatcher,
	ReactCurrentBatchConfig,
	ReactCurrentOwner,
	// Used by renderers to avoid bundling object-assign twice in UMD bundles:
	assign,
	};

	if (__DEV__) {
	ReactSharedInternals.ReactDebugCurrentFrame = ReactDebugCurrentFrame;
	ReactSharedInternals.ReactCurrentActQueue = ReactCurrentActQueue;
	}

Conversation

bvaughn commented Aug 17, 2021 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Testing

Uh oh!

sizebot commented Aug 17, 2021

Critical size changes

Significant size changes

Uh oh!

rickhanlonii commented Aug 18, 2021

Uh oh!

bvaughn commented Aug 18, 2021 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

5 participants

bvaughn commented Aug 17, 2021 •

edited

Loading

bvaughn commented Aug 18, 2021 •

edited

Loading