-
Notifications
You must be signed in to change notification settings - Fork 47k
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
[Fiber] Make DevTools Config use Static Injection #30522
Conversation
We shouldn't use meta programming like spread that gets deopted.
No need to have a bunch of `fieldName: null,` in the production builds.
Lets third parties specify their own version of their library separate from the reconciler.
The latest updates on your projects. Learn more about Vercel for Git ↗︎
|
bb3c784
to
4919b65
Compare
export const extraDevToolsConfig = { | ||
getInspectorDataForInstance, | ||
getInspectorDataForViewTag, | ||
getInspectorDataForViewAtPoint, | ||
}; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This will require some changes on React Native side before the sync
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Why is that? I tried to keep it the same as far as what React Native observes.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
You are right, I thought we are calling <renderer>.rendererConfig.getInspectorDataForViewAtPoint
on RN side, but we actually use it from RDT global hook, so should be perfectly fine
We use static dependency injection. We shouldn't use this dynamic dependency injection we do for DevTools internals. There's also meta programming like spreading and stuff that isn't needed. This moves the config from `injectIntoDevTools` to the FiberConfig so it can be statically resolved. Closure Compiler has some trouble generating optimal code for this anyway so ideally we'd refactor this further but at least this is better and saves a few bytes and avoids some code paths (when minified). DiffTrain build for [146df7c](146df7c)
We use static dependency injection. We shouldn't use this dynamic dependency injection we do for DevTools internals. There's also meta programming like spreading and stuff that isn't needed. This moves the config from `injectIntoDevTools` to the FiberConfig so it can be statically resolved. Closure Compiler has some trouble generating optimal code for this anyway so ideally we'd refactor this further but at least this is better and saves a few bytes and avoids some code paths (when minified). DiffTrain build for commit 146df7c.
We use static dependency injection. We shouldn't use this dynamic dependency injection we do for DevTools internals. There's also meta programming like spreading and stuff that isn't needed.
This moves the config from
injectIntoDevTools
to the FiberConfig so it can be statically resolved.Closure Compiler has some trouble generating optimal code for this anyway so ideally we'd refactor this further but at least this is better and saves a few bytes and avoids some code paths (when minified).