-
Notifications
You must be signed in to change notification settings - Fork 55
Add electron devtools installer back #2437
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
base: trunk
Are you sure you want to change the base?
Conversation
📊 Performance Test ResultsComparing 596195a vs trunk site-editor
site-startup
Results are median values from multiple test runs. Legend: 🟢 Improvement (faster) | 🔴 Regression (slower) | ⚪ No change |
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.
I was able to start the app without seeing any errors.
I am not sure if this was happening previously, but starting the app with devtools already opened doesn't show React DevTools.
I remember that previously we had this workaround: https://github.com/Automattic/studio/pull/1379/changes#diff-a2a171449d862fe29692ce031981047d7ab755ae7f84c707aef80701b3ea0c80R105
Maybe we still need that?
Doing a refresh ( cmd + R) displays the React DevTools as expected
package.json
Outdated
| }, | ||
| "scripts": { | ||
| "prestart": "npm run cli:build", | ||
| "setup:devtools": "ts-node ./scripts/setup-dev-extensions.ts", |
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.
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.
Good catch! It seems that the change from another PR slipped in. I removed this line.
That code was still there. I adjusted it, but I'm unsure if it helps - it still required me to run CMD+R to refresh it at least one. On subsequent runs, it shows without that refresh. |
Thanks for following up on that! |

Related issues
Proposed Changes
PR #2307 removed
electron-devtools-installerdue to deprecated Electron APIs, replacing it with a custom implementation. However, this broke the automatic extension installation for new developers.This PR takes a different approach by patching
electron-devtools-installerto use the new Electron APIs (session.extensions.*) instead of the deprecated ones (session.*). This solution:electron-devtools-installerpackageTesting Instructions
Pre-merge Checklist