Skip to content
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

Switch from webpack to esbuild for bundling webviews #2914

Merged
merged 1 commit into from
May 29, 2023

Conversation

datho7561
Copy link
Contributor

This PR uses esbuild instead of webpack to bundle the webviews. This dramatically reduces the time it takes to bundle the webviews, from >50 seconds to <1 second.

This PR also modifies the npm scripts and VS Code launch configurations to always recompile the webviews before launching.

I also set up eslint to be run before launching the extension. If the extension seems slower to launch in debug mode, this is why.

Closes #2875

Signed-off-by: David Thompson davthomp@redhat.com

@codecov
Copy link

codecov bot commented May 26, 2023

Codecov Report

Patch coverage: 79.31% and project coverage change: +0.31 🎉

Comparison is base (00f8659) 36.78% compared to head (6f2b9fb) 37.10%.

Additional details and impacted files
@@            Coverage Diff             @@
##             main    #2914      +/-   ##
==========================================
+ Coverage   36.78%   37.10%   +0.31%     
==========================================
  Files          53       54       +1     
  Lines        3681     3652      -29     
  Branches      715      716       +1     
==========================================
+ Hits         1354     1355       +1     
+ Misses       2327     2297      -30     
Impacted Files Coverage Δ
.../webview/create-service/createServiceViewLoader.ts 31.25% <50.00%> (+9.51%) ⬆️
src/webview/git-import/gitImportLoader.ts 16.66% <50.00%> (+0.81%) ⬆️
src/webview/devfile-registry/registryViewLoader.ts 31.32% <75.00%> (+0.21%) ⬆️
src/webview/common-ext/utils.ts 77.77% <77.77%> (ø)
src/webview/log/LogViewLoader.ts 26.92% <80.00%> (+5.71%) ⬆️
src/webview/helm-chart/helmChartLoader.ts 28.37% <87.50%> (+2.45%) ⬆️
src/webview/cluster/clusterViewLoader.ts 9.58% <88.88%> (+0.64%) ⬆️
src/webview/welcome/welcomeViewLoader.ts 66.66% <100.00%> (-3.83%) ⬇️

... and 1 file with indirect coverage changes

☔ View full report in Codecov by Sentry.
📢 Do you have feedback about the report comment? Let us know in this issue.

@datho7561 datho7561 marked this pull request as ready for review May 26, 2023 17:43
@datho7561 datho7561 marked this pull request as draft May 26, 2023 19:13
@datho7561
Copy link
Contributor Author

I realized that type checking is not being performed on the webview files. I'm going to make a quick change to fix that.

@datho7561
Copy link
Contributor Author

Just ran into this: microsoft/vscode#70283. Prelaunch tasks that use dependsOn with a background task cannot prevent debug from launching. This is a bit of a set back. I think my work around for this will be not running eslint as a part of the prelaunch tasks, and running the rest of the tasks synchronously every time launch is triggered.

@rgrunber
Copy link
Member

Tried it out and seems to be working well for me!

Copy link
Collaborator

@msivasubramaniaan msivasubramaniaan left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

This PR uses esbuild instead of webpack to bundle the webviews.
This dramatically reduces the time it takes to bundle the webviews,
from >50 seconds to <1 second.

This PR also modifies the npm scripts and VS Code launch configurations
to always recompile the webviews before launching.

I also set up eslint to be run before launching the extension.
If the extension seems slower to launch in debug mode, this is why.

Closes redhat-developer#2875

Signed-off-by: David Thompson <davthomp@redhat.com>
@rgrunber rgrunber merged commit 868509c into redhat-developer:main May 29, 2023
@datho7561 datho7561 deleted the 2875-esbuild-webviews branch May 29, 2023 19:59
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Investigate decreasing npm run compile times
3 participants