workaround electron-rebuild bug
#10429
Conversation
paul-marechal
added
bug
paul-marechal
force-pushed
the
mp/rebuild-workaround
branch
from
01a5368 to
068c907
Compare
|
Just updated the signals to listen to: I mixed up |
paul-marechal
force-pushed
the
mp/rebuild-workaround
branch
from
068c907 to
9a85fa0
Compare
There was a problem hiding this comment.
I confirmed that the changes work well:
-
yarn browser rebuild -
yarn electron rebuild -
npx theia rebuild:electron(root) works well -
npx theia rebuild:browser(root) works well - starting the browser app after the browser rebuild works
- starting the electron app after the electron rebuild works
- performing
ctrl+cwill revert the rebuild properly foryarn {app} rebuild - performing
ctrl+cwill revert the rebuild properly fornpx theia rebuild:{app}
|
@vince-fugnitto the value of this PR lies in being able to do |
See electron/rebuild#888 The way we invoke `electron-rebuild` should allow Theia application developers to run `theia rebuild:*` from the root of their monorepos, but because of a bug in `electron-rebuild` it will only build dependencies listed in the `package.json` file located in `cwd`. Implement a workaround that modifies or creates a `package.json` file to list the dependencies to rebuild. Add exit hooks to cleanup in case users do `Ctrl+C` while theia rebuild is running so that we don't pollute user workspaces.
paul-marechal
force-pushed
the
mp/rebuild-workaround
branch
from
9a85fa0 to
64d7a37
Compare
@paul-marechal both :) I updated my comment #10429 (review). |
There was a problem hiding this comment.
Only very minor punctuation comments on the code.
More substantively, I'm able to run npx theia rebuild:* from the Theia repo root on master (29ebe4c), so if that's the main objective of this PR, I'm not sure it's necessary. Is there something I'm missing there?
There was a problem hiding this comment.
My comments re: punctuation stand, though they don't really need to be addressed. My comment re: ability to run npx theia rebuild:* in the current state of master appears to have been erroneous: that runs afoul of the electron rebuild bug, and a module fails to self register.
See electron/rebuild#888
The way we invoke
electron-rebuildshould allow Theia applicationdevelopers to run
theia rebuild:*from the root of their monorepos,but because of a bug in
electron-rebuildit will only builddependencies listed in the
package.jsonfile located incwd.Implement a workaround that modifies or creates a
package.jsonfile tolist the dependencies to rebuild.
Add exit hooks to cleanup in case users do
Ctrl+Cwhile theia rebuildis running so that we don't pollute user workspaces.
How to test
npx theia rebuild:electronandnpx theia rebuild:browserfrom Theia's monorepo root. It should properly rebuild modules for the specified target.Ctrl+Cwhiletheia rebuild:electronis running should revert the modules back to the browser version.Ctrl+Cwhiletheia rebuild:browseris running should not prevent the process from finishing.Review checklist
Reminder for reviewers