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

Refactor Host Config Infra (getting rid of .inline*.js) #18240

Merged
merged 5 commits into from Mar 7, 2020

Conversation

@sebmarkbage
Copy link
Member

sebmarkbage commented Mar 6, 2020

I kept forgetting what the various .inline files are supposed to do, and it made it hard to add more combinations.

The first thing this does is getting rid of the various renderer depending on the indirection pseudo entry point (inline.dom.js etc). Instead we just deep require the reconciler. We do this for other files anyway and there's nothing really preventing this. Instead, we can just use the entry-point of the actual renderer as the jest hook to initialize the correct host config.

The actual react-reconciler/index.js entry point should only represent the public API (currently a function wrapper published to npm). Not used by us.

There are no longer any untyped files so we don't need the special inline-typed form. The untyped files caused some uncertainty since once an untyped file gets into the system it leaks the any all over the place. It also wasn't a sufficient approach (I had some hacks to hide DOM).

Instead, for Flow, in this approach we explicitly select which paths we want to check for each renderer. The inverse gets ignored. So every path is still covered by some renderer.

These ignores are not untyped so if a dependency accidentally tries to reach into something that isn't covered, then that will be an error. Not silently leak "any".

@sebmarkbage sebmarkbage requested review from gaearon and acdlite Mar 6, 2020
@sebmarkbage sebmarkbage changed the title Refactor Host Config (getting rid of .inline*.js) Refactor Host Config Infra (getting rid of .inline*.js) Mar 6, 2020
@codesandbox

This comment has been minimized.

Copy link

codesandbox bot commented Mar 6, 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 fd4699d:

Sandbox Source
heuristic-hertz-hmurl Configuration
@sizebot

This comment has been minimized.

Copy link

sizebot commented Mar 6, 2020

No significant bundle size changes to report.

Size changes (experimental)

Generated by 🚫 dangerJS against fd4699d

@sizebot

This comment has been minimized.

Copy link

sizebot commented Mar 6, 2020

No significant bundle size changes to report.

Size changes (stable)

Generated by 🚫 dangerJS against fd4699d

@sebmarkbage

This comment has been minimized.

Copy link
Member Author

sebmarkbage commented Mar 6, 2020

Oh, this also delete react-reconciler/persistent since that's just a flag on host config now.

sebmarkbage added 5 commits Mar 6, 2020
This no longer makes any sense because it react-reconciler takes
supportsMutation or supportsPersistence as options. It's no longer based
on feature flags.
We now explicitly list which paths we want to be checked by a renderer.
For every other renderer config we ignore those paths.

Nothing is "any" typed. So if some transitive dependency isn't reachable
it won't be accidentally "any" that leaks.
@sebmarkbage sebmarkbage force-pushed the sebmarkbage:rminlinewtf branch from 4e9101e to fd4699d Mar 6, 2020
'react-dom',
'react-dom/unstable-fizz',
'react-dom/unstable-fizz.node',
'react-dom/src/server/ReactDOMFizzServerNode.js', // react-dom/unstable-fizz.node

This comment has been minimized.

Copy link
@sebmarkbage

sebmarkbage Mar 6, 2020

Author Member

These are explicitly called out so they get ignored in "dom-browser" and vice versa. Which otherwise has a lot of overlapping paths.

@gaearon
gaearon approved these changes Mar 6, 2020
Copy link
Member

gaearon left a comment

I don't remember why I added any of this. Delete it

@sebmarkbage sebmarkbage merged commit 7a1691c into facebook:master Mar 7, 2020
31 checks passed
31 checks passed
ci/circleci: build Your tests passed on CircleCI!
Details
ci/circleci: build_devtools_and_process_artifacts Your tests passed on CircleCI!
Details
ci/circleci: build_experimental Your tests passed on CircleCI!
Details
ci/circleci: flow Your tests passed on CircleCI!
Details
ci/circleci: lint Your tests passed on CircleCI!
Details
ci/circleci: lint_build Your tests passed on CircleCI!
Details
ci/circleci: process_artifacts Your tests passed on CircleCI!
Details
ci/circleci: process_artifacts_experimental Your tests passed on CircleCI!
Details
ci/circleci: setup Your tests passed on CircleCI!
Details
ci/circleci: sizebot Your tests passed on CircleCI!
Details
ci/circleci: sizebot_experimental Your tests passed on CircleCI!
Details
ci/circleci: test_build Your tests passed on CircleCI!
Details
ci/circleci: test_build_experimental Your tests passed on CircleCI!
Details
ci/circleci: test_build_prod Your tests passed on CircleCI!
Details
ci/circleci: test_build_prod_experimental Your tests passed on CircleCI!
Details
ci/circleci: test_devtools Your tests passed on CircleCI!
Details
ci/circleci: test_dom_fixtures Your tests passed on CircleCI!
Details
ci/circleci: test_source Your tests passed on CircleCI!
Details
ci/circleci: test_source_experimental Your tests passed on CircleCI!
Details
ci/circleci: test_source_persistent Your tests passed on CircleCI!
Details
ci/circleci: test_source_prod Your tests passed on CircleCI!
Details
ci/circleci: test_source_prod_experimental Your tests passed on CircleCI!
Details
ci/circleci: test_source_www Your tests passed on CircleCI!
Details
ci/circleci: test_source_www_experimental Your tests passed on CircleCI!
Details
ci/circleci: test_source_www_prod Your tests passed on CircleCI!
Details
ci/circleci: test_source_www_prod_experimental Your tests passed on CircleCI!
Details
ci/circleci: test_source_www_variant Your tests passed on CircleCI!
Details
ci/circleci: test_source_www_variant_experimental Your tests passed on CircleCI!
Details
ci/circleci: test_source_www_variant_prod Your tests passed on CircleCI!
Details
ci/circleci: test_source_www_variant_prod_experimental Your tests passed on CircleCI!
Details
ci/codesandbox Building packages succeeded.
Details
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Linked issues

Successfully merging this pull request may close these issues.

None yet

4 participants
You can’t perform that action at this time.