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

Move some files out of /shared and rename to upper case #18363

Merged
merged 5 commits into from Mar 21, 2020

Conversation

sebmarkbage
Copy link
Collaborator

This moves some stuff out of shared and into their respective packages.

I changed the isomorphic "creator" types to use the same convention (upper case) with named exports. It was inconsistent before.

Things that are Fiber specific can live in the reconciler because any individual renderer is allowed to deep link into react-reconciler. At least ours are.

@facebook-github-bot facebook-github-bot added CLA Signed React Core Team Opened by a member of the React Core Team labels Mar 21, 2020
…med exports

We're somewhat inconsistent here between e.g. ReactLazy and memo.

Let's pick one.

This also moves the responder, fundamental, scope creators from shared
since they're isomorphic and same as the other creators.
Individual renderers are allowed to deep require into the reconciler.
react-interactions is right now dom specific (it wasn't before) so we can
type check it together with other dom stuff. Avoids the need for
a shared ReactDOMTypes to be checked by RN for example.
@codesandbox-ci
Copy link

codesandbox-ci bot commented Mar 21, 2020

This pull request is automatically built and testable in CodeSandbox.

To see build info of the built libraries, click here or the icon next to each commit SHA.

Latest deployment of this branch, based on commit c581aa3:

Sandbox Source
dazzling-voice-8s6xc Configuration

@sizebot
Copy link

sizebot commented Mar 21, 2020

Details of bundled changes.

Comparing: a600408...c581aa3

react-noop-renderer

File Filesize Diff Gzip Diff Prev Size Current Size Prev Gzip Current Gzip ENV
react-noop-renderer.development.js -1.5% -3.1% 29.88 KB 29.43 KB 5.77 KB 5.6 KB NODE_DEV
react-noop-renderer.production.min.js -0.7% -1.3% 12.3 KB 12.22 KB 3.6 KB 3.55 KB NODE_PROD
react-noop-renderer-server.production.min.js 0.0% -0.2% 840 B 840 B 472 B 471 B NODE_PROD
react-noop-renderer-persistent.development.js -1.5% -3.1% 29.79 KB 29.34 KB 5.76 KB 5.59 KB NODE_DEV
react-noop-renderer-persistent.production.min.js -0.7% -1.3% 12.26 KB 12.18 KB 3.59 KB 3.55 KB NODE_PROD
react-noop-renderer-flight-server.development.js 0.0% -0.1% 1.78 KB 1.78 KB 824 B 823 B NODE_DEV

react-reconciler

File Filesize Diff Gzip Diff Prev Size Current Size Prev Gzip Current Gzip ENV
react-reconciler-reflection.production.min.js 0.0% -0.1% 2.57 KB 2.57 KB 1.09 KB 1.08 KB NODE_PROD
react-reconciler.development.js +0.1% +0.1% 574.9 KB 575.4 KB 121.37 KB 121.54 KB NODE_DEV
react-reconciler.production.min.js 🔺+0.2% 🔺+0.3% 75.99 KB 76.18 KB 22.52 KB 22.58 KB NODE_PROD

Size changes (stable)

Generated by 🚫 dangerJS against c581aa3

@sizebot
Copy link

sizebot commented Mar 21, 2020

Details of bundled changes.

Comparing: a600408...c581aa3

react-noop-renderer

File Filesize Diff Gzip Diff Prev Size Current Size Prev Gzip Current Gzip ENV
react-noop-renderer-server.development.js 0.0% -0.1% 1.6 KB 1.6 KB 754 B 753 B NODE_DEV
react-noop-renderer-persistent.development.js -1.5% -3.1% 29.8 KB 29.35 KB 5.77 KB 5.59 KB NODE_DEV
react-noop-renderer-persistent.production.min.js -0.7% -1.3% 12.28 KB 12.19 KB 3.6 KB 3.55 KB NODE_PROD
react-noop-renderer.development.js -1.5% -3.1% 29.89 KB 29.44 KB 5.78 KB 5.6 KB NODE_DEV
react-noop-renderer-flight-client.development.js 0.0% -0.2% 1.2 KB 1.2 KB 625 B 624 B NODE_DEV
react-noop-renderer.production.min.js -0.7% -1.3% 12.31 KB 12.23 KB 3.61 KB 3.56 KB NODE_PROD

react-reconciler

File Filesize Diff Gzip Diff Prev Size Current Size Prev Gzip Current Gzip ENV
react-reconciler.development.js +0.1% +0.1% 597.36 KB 597.85 KB 125.61 KB 125.77 KB NODE_DEV
react-reconciler.production.min.js 🔺+0.2% 🔺+0.3% 79.09 KB 79.27 KB 23.26 KB 23.32 KB NODE_PROD

Size changes (experimental)

Generated by 🚫 dangerJS against c581aa3

Otherwise Noop can't access it since it's not allowed deep requires.
@trueadm
Copy link
Contributor

trueadm commented Mar 21, 2020

I believe the RN sync script pulls some bits out of shared, so we should make sure that those bits still work. However, it might only be ReactTypes which I don't think you changed here.

@sebmarkbage
Copy link
Collaborator Author

sebmarkbage commented Mar 21, 2020

Yea it's only ReactTypes and ReactNativeTypes. However, I'll separately open a diff to stop syncing ReactTypes. We shouldn't be using that in RN because it's not consistent with the upstream Flow types. That causes issues and all current imports were incorrect and has been codemodded so they're now unused.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
CLA Signed React Core Team Opened by a member of the React Core Team
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

5 participants