-
Notifications
You must be signed in to change notification settings - Fork 2.5k
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
Generate webpack config for backend bundling #12412
Conversation
a7b70d7
to
0bc17a4
Compare
I could not start the electron example app from VS Code using the I did:
Click to see what I got when I started Theia from Code:
|
7d2ff77
to
e4999e6
Compare
@kittaakos I've noticed that the webpack plugin doesn't work well when building multiple projects at the same time. I've adapted it a bit. It should work as expected now 👍 |
Thank you for the ping and update, @msujew. I tried it yesterday, right after you had updated the PR, but I had the same problem.
I have checked the files, and the generated modules were all there. I will give it another try with a fresh clone soon. |
@kittaakos I've just identified the issue. It's supposed to go trough the "real" |
c997bba
to
60d7ea9
Compare
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 experimented with this PR using a non-trivial Theia-based application on Windows 11:
- Backported the change to Theia 1.34.2
- Consumed Theia as source in the Theia-based application
We have some manual file system operations in our extensions using __dirname
. These were then failing, so I removed them for now. Maybe the webpack build could warn when such constructs are encountered.
Besides that problem and a small hiccup with the generated webpack file I got the Browser variant working. I was successfully able remove the node_modules directory and was still able to run the Theia backend 🎉
However I failed to get the Electron variant to run. For the build I had to add --max-old-space-size=4096
as the Webpack build now required more memory. But even with that out of the way and the build succeeding, Electron immediately crashed with a Javascript error in the main process: "Uncaught Exception: Error: No native build was found for platform=win32 arch=x64 runtime=electron abi=98 uv=1 libc=glibc node=16.5.0 electron=15.5.7 webpack=true loaded from: C:\theia-webpack\apps\electron\lib".
I tried to start the Electron variant with commands like node_modules/.bin/electron.cmd . --app-project-path=.
and adapted the main
path of the Electron app package to the lib/backend/electron-main.js
file.
Do I have to do something different? I was able to start the regular Electron build this way.
dev-packages/application-manager/src/generator/webpack-generator.ts
Outdated
Show resolved
Hide resolved
7b2036a
to
6d9266a
Compare
Hi, I have checked out this branch multiple times but still cannot start the electron example application from VS Code. What am I doing wrong? I check out the branch, run click to see full debug console output
Thank you! |
@kittaakos I'm wondering right now how to improve on this. Any idea? |
This is working great now. I can debug the Theia backend too. That is a very nice feature. Thank you! I will take a deeper look this week. |
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.
Changes look good with only a few comments. I can run the app and debug from TS sources.
dev-packages/application-manager/src/generator/webpack-generator.ts
Outdated
Show resolved
Hide resolved
@paul-marechal Thanks, yeah that irked me quite a bit as well. I've moved all backend components into a |
@paul-marechal FYI I noticed that webviews weren't working as expected with that approach. They should work now (see a105ab3). |
If you want my opinion we should have a |
@paul-marechal Generating into Note that I moved some stuff again from |
@sdirix Right, that is necessary in order for the bundled app to work. Thank you for the feedback by the way. I've done a few changes to the electron build, and it might be more stable now. Are you interested in retesting the issue you were experiencing? Note that a rebuild of the native dependencies needs to be triggered before running |
3293b01
to
efecdb0
Compare
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.
Changes LGTM, I can run the examples apps without apparent errors.
Some minor comments left before merging.
About having to do |
@paul-marechal Yes, I agree. I'll update the build scripts 👍 Thanks for the review :) |
2d6199d
to
044517c
Compare
Hello, how do I disable bundling the backend code? Thank you! |
Hi @msujew, I did tried to comment out the line that you have pointed out. And re-run yarn and rebuild the browser example. Calling __dirname in one of my extension, it returns /examples/browser/lib/backend. Using theia v1.36, __dirname returns the lib folder of my extension and not the examples/browser/lib/backend folder. One additional difference I observe, require.resolve() returns the location of the node_modules folder within the browser example, it used to return the location of the node_module where the exist on disk. It exists in the root node_modules folder. |
@pchuong Since you already built the bundled backend once, you'll need to delete the |
i got same error, please tell me how to solve this |
What it does
Closes #12391
Generates in addition to the existing
gen-webpack.config.js
anothergen-webpack.node.config.js
. This configuration can be used to also bundle the applications backend. Note that this change completely optional, and devs can still opt for running a non-bundled backend.In order to simplify copying & and modifying native executables, a new package is introduced
@theia/native-webpack-plugin
. It handles webpack bundling for special, hard to bundle native dependencies such as@vscode/ripgrep
,drivelist
,native-keymap
andnode-pty
.How to test
This change affects both the browser and electron version of Theia. Please perform all testing steps on both platforms:
Optional Bundling
--mode=production
should produce a minified production build that also works as expected.webpack.config.js
and clean the repo. Runningtheia build
should result in a non-bundled backend that still starts/works as expected.No dependencies on
node_modules
node_modules
(except for theelectron
dependency).start
scripts to something like thisnode ./lib/backend/main.js
.Debugging
Run all relevant debug configurations and confirm that debugging the application (especially the backend) works as expected.
Electron Application Bundling
Note: I haven't performed this step myself yet due to a lack of time. It might not work as expected yet.
Review checklist
Reminder for reviewers