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 getIsHydrating from FB fork to Experimental flag #18012

Closed
wants to merge 1 commit into from

Conversation

gaearon
Copy link
Collaborator

@gaearon gaearon commented Feb 10, 2020

This has no actual impact except that getIsHydrating is on the secrets object for everyone, and not just FB. This would let us completely get rid of the forked FB-specific entry point in the modern version. Instead of the forked entry point, we will use feature flags to remove individual features.

@facebook-github-bot facebook-github-bot added React Core Team Opened by a member of the React Core Team CLA Signed labels Feb 10, 2020
@sizebot
Copy link

sizebot commented Feb 10, 2020

Warnings
⚠️ Could not find build artifacts for base commit: ab7b83a

Size changes (experimental)

Generated by 🚫 dangerJS against a73fbc3

@codesandbox-ci
Copy link

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 a73fbc3:

Sandbox Source
quizzical-mclaren-yfwgg Configuration

@sizebot
Copy link

sizebot commented Feb 10, 2020

No significant bundle size changes to report.

Size changes (stable)

Generated by 🚫 dangerJS against a73fbc3

Copy link
Collaborator

@sebmarkbage sebmarkbage left a comment

Choose a reason for hiding this comment

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

Do we even need this? Should we disable the user space suspensey images? Since we're blocked on the upstreamed version anyway, which won't need this.

@@ -179,6 +180,8 @@ if (exposeConcurrentModeAPIs) {
queueExplicitHydrationTarget(target);
}
};

ReactDOM.__SECRET_INTERNALS_DO_NOT_USE_OR_YOU_WILL_BE_FIRED.getIsHydrating = getIsHydrating;
Copy link
Collaborator

Choose a reason for hiding this comment

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

It's very important that this isn't exposed in the CM release. It won't be needed by the future API neither.

Instead, put it behind a specific feature flag that we won't enable.

@gaearon
Copy link
Collaborator Author

gaearon commented Feb 10, 2020

Ahh I thought this was for hero tracing or something like this. Okay I'll disable and just remove this.

@gaearon gaearon mentioned this pull request Feb 11, 2020
@gaearon
Copy link
Collaborator Author

gaearon commented Feb 12, 2020

Ain't needed.

@gaearon gaearon closed this Feb 12, 2020
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.

4 participants