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

Reduce risk of leaving shipped Hooks as nullable on Dispatcher #32068

Merged
merged 1 commit into from
Jan 14, 2025

Conversation

eps1lon
Copy link
Collaborator

@eps1lon eps1lon commented Jan 14, 2025

By grepping for the feature flag you're now directed to the Dispatcher type. Members are only nullable if they're behind a feature flag so we should make them non-nullable when the flag ships. There's still the possibility that types are not updated when the flag is on everywhere but cleanup should flush it out.

@facebook-github-bot facebook-github-bot added CLA Signed React Core Team Opened by a member of the React Core Team labels Jan 14, 2025
@@ -834,6 +834,8 @@ export const HooksDispatcher: Dispatcher = supportsClientAPIs
useActionState,
useFormState: useActionState,
useHostTransitionStatus,
useMemoCache,
useCacheRefresh,
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

This is available everywhere else.

@react-sizebot
Copy link

react-sizebot commented Jan 14, 2025

Comparing: f0edf41...d12a451

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.js = 6.68 kB 6.68 kB +0.11% 1.83 kB 1.83 kB
oss-stable/react-dom/cjs/react-dom-client.production.js = 514.24 kB 514.24 kB = 91.73 kB 91.73 kB
oss-experimental/react-dom/cjs/react-dom.production.js = 6.69 kB 6.69 kB +0.11% 1.83 kB 1.83 kB
oss-experimental/react-dom/cjs/react-dom-client.production.js = 556.18 kB 556.18 kB = 98.72 kB 98.72 kB
facebook-www/ReactDOM-prod.classic.js = 595.79 kB 595.79 kB = 104.85 kB 104.85 kB
facebook-www/ReactDOM-prod.modern.js = 586.21 kB 586.21 kB = 103.30 kB 103.30 kB

Significant size changes

Includes any change greater than 0.2%:

(No significant changes)

Generated by 🚫 dangerJS against 858c71f

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.

It really shouldn't be added everywhere since it's not an exposed API there's no reason to ship the bytes. A lot of the Cache stuff isn't gated properly.

@eps1lon
Copy link
Collaborator Author

eps1lon commented Jan 14, 2025

It really shouldn't be added everywhere since it's not an exposed API there's no reason to ship the bytes. A lot of the Cache stuff isn't gated properly.

I think we just accidentally shipped this because it was part of enableCache which was removed in #31778.

Should we reintroduce a new flag that's only on for experimental (React Devtools needs it)? Though unstable_getCacheForType was also fully unflagged which is odd since it's still marked as unstable_.

@eps1lon eps1lon merged commit 886c5ad into facebook:main Jan 14, 2025
186 checks passed
github-actions bot pushed a commit that referenced this pull request Jan 14, 2025
github-actions bot pushed a commit to code/lib-react that referenced this pull request Jan 15, 2025
github-actions bot pushed a commit to AreaLayer/react that referenced this pull request Jan 15, 2025
github-actions bot pushed a commit to AreaLayer/react that referenced this pull request Jan 15, 2025
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.

5 participants