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

Make esbuild to produce bundled & minified VSIX #4584

Merged

Conversation

vrubezhny
Copy link
Contributor

@vrubezhny vrubezhny commented Oct 25, 2024

This fix makes 'esbuild' to produce the bundled and minified VSIX extension archive,
free of unneeded dependency modules, when the packaging is started with vsce package.

On other hand, when building with npm install && npm run build and testing with npm test
the extension file structure is kept unchange and the trunspiled scripts aren't minified,
so the unit testing and coverage tests work as usual.

Note: npm install is needed to be executed after executing vsce package as the last one
clears the node_modules/ of the dependencies not needed in production.

Fixes #4226

@vrubezhny vrubezhny marked this pull request as draft October 25, 2024 04:28
@vrubezhny vrubezhny force-pushed the fix-esbuild-to-produce-bundle branch 15 times, most recently from 6ebcdf0 to cc8dbde Compare November 11, 2024 16:12
@vrubezhny vrubezhny force-pushed the fix-esbuild-to-produce-bundle branch 9 times, most recently from 44c567e to 90b3305 Compare November 13, 2024 16:40
@codecov-commenter
Copy link

codecov-commenter commented Nov 13, 2024

Codecov Report

Attention: Patch coverage is 58.33333% with 140 lines in your changes missing coverage. Please review.

Project coverage is 44.51%. Comparing base (da60441) to head (cc4cefc).
Report is 674 commits behind head on main.

Files with missing lines Patch % Lines
src/webview/cluster/clusterViewLoader.ts 19.04% 51 Missing ⚠️
src/webview/devfile-registry/registryViewLoader.ts 40.90% 13 Missing ⚠️
...w/helm-manage-repository/manageRepositoryLoader.ts 33.33% 12 Missing ⚠️
src/webview/helm-chart/helmChartLoader.ts 59.25% 11 Missing ⚠️
src/helm/manageRepository.ts 28.57% 10 Missing ⚠️
...ebview/create-deployment/createDeploymentLoader.ts 47.05% 9 Missing ⚠️
src/helm/helm.ts 38.46% 8 Missing ⚠️
src/serverlessFunction/manageRepository.ts 33.33% 6 Missing ⚠️
src/k8s/console.ts 66.66% 5 Missing ⚠️
src/explorer.ts 33.33% 4 Missing ⚠️
... and 11 more
Additional details and impacted files
@@             Coverage Diff             @@
##             main    #4584       +/-   ##
===========================================
+ Coverage   32.37%   44.51%   +12.14%     
===========================================
  Files          85      100       +15     
  Lines        6505     8170     +1665     
  Branches     1349     1707      +358     
===========================================
+ Hits         2106     3637     +1531     
- Misses       4399     4533      +134     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.


🚨 Try these New Features:

@vrubezhny vrubezhny force-pushed the fix-esbuild-to-produce-bundle branch 3 times, most recently from 2365001 to 39f4aeb Compare November 14, 2024 22:03
src/vscommand.ts Outdated Show resolved Hide resolved
@vrubezhny vrubezhny force-pushed the fix-esbuild-to-produce-bundle branch 3 times, most recently from 39b38f8 to 8f1e748 Compare November 18, 2024 15:39
@vrubezhny vrubezhny requested a review from datho7561 November 18, 2024 15:55
Copy link
Contributor

@datho7561 datho7561 left a comment

Choose a reason for hiding this comment

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

Looking almost ready for merge. Found some potentially unused code

src/util/utils.ts Outdated Show resolved Hide resolved
@vrubezhny vrubezhny force-pushed the fix-esbuild-to-produce-bundle branch from 8f1e748 to 2fb7ca9 Compare November 18, 2024 17:19
@vrubezhny vrubezhny requested a review from datho7561 November 18, 2024 17:20
datho7561
datho7561 previously approved these changes Nov 18, 2024
Copy link
Contributor

@datho7561 datho7561 left a comment

Choose a reason for hiding this comment

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

Looks good and works for me. Thanks Victor!

@vrubezhny
Copy link
Contributor Author

@lgrossma Could you please take a look at UI tests:

Packaged: /home/runner/work/vscode-openshift-tools/vscode-openshift-tools/vscode-openshift-connector-1.16.0.vsix (421 files, 175.44 MB)
Installing extensions...
Extension 'vscode-openshift-connector-1.16.0.vsix' was successfully installed.
Installing extensions...
Extension 'redhat.vscode-redhat-account' is already installed.
Loading mocha configuration from test/ui/.mocharc.js


  0 passing (1ms)

@vrubezhny vrubezhny marked this pull request as draft November 19, 2024 15:10
@vrubezhny vrubezhny force-pushed the fix-esbuild-to-produce-bundle branch 5 times, most recently from 90e09bc to cf3a5a1 Compare November 20, 2024 17:46
@vrubezhny
Copy link
Contributor Author

The Public UI tests are restored.

@vrubezhny vrubezhny marked this pull request as ready for review November 20, 2024 18:07
@vrubezhny vrubezhny force-pushed the fix-esbuild-to-produce-bundle branch from cf3a5a1 to cc4cefc Compare November 20, 2024 18:26
This fix makes 'esbuild' to produce the bundled and minified VSIX extension archive,
free of unneeded dependency modules, when the packaging is started with `vsce package`.

On other hand, when building with `npm install && npm run build` and testing with `npm test`
the extension file structure is kept unchange and the trunspiled scripts aren't minified,
so the unit testing and coverage tests work as usual.

Note: `npm install` is needed to be executed after executing `vsce package` as the last one
clears the `node_modules/` of the depencencies not needed in production.

Fixes redhat-developer#4226

Signed-off-by: Victor Rubezhny <[email protected]>
@vrubezhny vrubezhny force-pushed the fix-esbuild-to-produce-bundle branch from cc4cefc to 8201122 Compare November 20, 2024 23:05
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

@vrubezhny
Copy link
Contributor Author

@datho7561 @msivasubramaniaan Thanks for the reviews and @lgrossma for helping with the tests!

@vrubezhny vrubezhny merged commit 3086516 into redhat-developer:main Nov 21, 2024
4 checks passed
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.

Extension is slow to activate
4 participants