-
Notifications
You must be signed in to change notification settings - Fork 46.7k
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
Don't use Spread in DevTools Injection #18277
Conversation
bundleType: devToolsConfig.bundleType, | ||
version: devToolsConfig.version, | ||
rendererPackageName: devToolsConfig.rendererPackageName, | ||
findFiberByHostInstance: devToolsConfig.findFiberByHostInstance, |
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 last line would be replaced with rendererConfig
if we do https://github.com/facebook/react/pull/18233/files#r391135037
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 ee144e1:
|
I believe this change should let us compile out the intermediate steps here but there's still something about how this is wired up that confuses the compiler until leave an unnecessary closure in place. I think it's because our use of closures in findHostInstanceByFiber. |
Related to this https://github.com/facebook/react/pull/18233/files#r391135037
We could encourage new arbitrary things to be added to this object through spread but that shares a namespace with our own things which risks us colliding and causing weird behavior. The Flow type defines that this is exact so to enforce that it is indeed exact, we can instead enumerate the values.
We really should be doing that everywhere in our internals, like we do with fiber cloning for example, and ideally avoid Object.assign as much as possible even in user facing APIs.
In DEV behavior I'm not as strict but this ships in PROD.