[DevTools] Remove renderer.js from extension build #26234
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
facebook-github-bot
added
CLA Signed
React Core Team
github-actions bot
pushed a commit
that referenced
this pull request
Mar 3, 2023
## Summary When looking into the compiled code of `installHook.js` of the extension build, I noticed that it actually includes the large `attach` function (from renderer.js). I don't think it was expected. This is because `hook.js` imports from `backend/console.js` which imports from `backend/renderer.js` for `getInternalReactConstants` A straightforward way is to extract function `getInternalReactConstants`. However, I think it's more simplified to just merge these two files and save the 361K renderer.js from the extension build since we have always been loading this code anyways. I changed the execution check from `__REACT_DEVTOOLS_ATTACH__ ` to the session storage. ## How did you test this change? Everything works normal in my local build. DiffTrain build for [fcf2187](fcf2187)
mondaychen
added a commit
that referenced
this pull request
Apr 6, 2023
This reverts commit fcf2187.
mondaychen
added a commit
that referenced
this pull request
Apr 7, 2023
## Summary - #26234 is reverted and replaced with a better approach - introduce a new global devtools variable to decouple the global hook's dependency on backend/console.js, and add it to react-devtools-inline and react-devtools-standalone With this PR, I want to introduce a new principle to hook.js: we should always be alert when editing this file and avoid importing from other files. In the past, we try to inline a lot of the implementation because we use `.toString()` to inject this function from the extension (we still have some old comments left). Although it is no longer inlined that way, it has became now more important to keep it clean as it is a de facto global API people are using (9.9K files contains it on Github search as of today). **File size change for extension:** Before: 379K installHook.js After: 21K installHook.js 363K renderer.js
kassens
pushed a commit
to kassens/react
that referenced
this pull request
Apr 17, 2023
…6563) ## Summary - facebook#26234 is reverted and replaced with a better approach - introduce a new global devtools variable to decouple the global hook's dependency on backend/console.js, and add it to react-devtools-inline and react-devtools-standalone With this PR, I want to introduce a new principle to hook.js: we should always be alert when editing this file and avoid importing from other files. In the past, we try to inline a lot of the implementation because we use `.toString()` to inject this function from the extension (we still have some old comments left). Although it is no longer inlined that way, it has became now more important to keep it clean as it is a de facto global API people are using (9.9K files contains it on Github search as of today). **File size change for extension:** Before: 379K installHook.js After: 21K installHook.js 363K renderer.js
EdisonVan
pushed a commit
to EdisonVan/react
that referenced
this pull request
Apr 15, 2024
…6563) ## Summary - facebook#26234 is reverted and replaced with a better approach - introduce a new global devtools variable to decouple the global hook's dependency on backend/console.js, and add it to react-devtools-inline and react-devtools-standalone With this PR, I want to introduce a new principle to hook.js: we should always be alert when editing this file and avoid importing from other files. In the past, we try to inline a lot of the implementation because we use `.toString()` to inject this function from the extension (we still have some old comments left). Although it is no longer inlined that way, it has became now more important to keep it clean as it is a de facto global API people are using (9.9K files contains it on Github search as of today). **File size change for extension:** Before: 379K installHook.js After: 21K installHook.js 363K renderer.js
bigfootjon
pushed a commit
that referenced
this pull request
Apr 18, 2024
## Summary - #26234 is reverted and replaced with a better approach - introduce a new global devtools variable to decouple the global hook's dependency on backend/console.js, and add it to react-devtools-inline and react-devtools-standalone With this PR, I want to introduce a new principle to hook.js: we should always be alert when editing this file and avoid importing from other files. In the past, we try to inline a lot of the implementation because we use `.toString()` to inject this function from the extension (we still have some old comments left). Although it is no longer inlined that way, it has became now more important to keep it clean as it is a de facto global API people are using (9.9K files contains it on Github search as of today). **File size change for extension:** Before: 379K installHook.js After: 21K installHook.js 363K renderer.js DiffTrain build for commit dd53658.
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Summary
When looking into the compiled code of
installHook.jsof the extension build, I noticed that it actually includes the largeattachfunction (from renderer.js). I don't think it was expected.This is because
hook.jsimports frombackend/console.jswhich imports frombackend/renderer.jsforgetInternalReactConstantsA straightforward way is to extract function
getInternalReactConstants. However, I think it's more simplified to just merge these two files and save the 361K renderer.js from the extension build since we have always been loading this code anyways.I changed the execution check from
__REACT_DEVTOOLS_ATTACH__to the session storage.How did you test this change?
Everything works normal in my local build.