chore: Bundle the extension using esbuild #421

cwahbong · 2024-10-20T02:05:34Z

This should not affect the existing build and test flow, but adding an additional step right before building vsix package.

The esbuild.js is copied from VSCode extension generator template then format fixed by prettier.

This reduces the package size from ~5MB -> ~250KB.

vogelsgesang

Thanks for this PR! Properly bundling and minifying the extensions has been on my TODO list for a long time already.

I know that there were discussions on "should we build this extension via Bazel", but afaik they stalled. As such, I would be happy to move on with this PR. We can later still switch to Bazel, in case somebody is willing to put in the effort for this switch

.vscodeignore

package.json

vogelsgesang

Looks good to me. I am still waiting before merging this, to also give others a chance to review

scripts/package.sh

cwahbong · 2024-10-20T12:47:42Z

Thanks for this PR! Properly bundling and minifying the extensions has been on my TODO list for a long time already.

I know that there were discussions on "should we build this extension via Bazel", but afaik they stalled. As such, I would be happy to move on with this PR. We can later still switch to Bazel, in case somebody is willing to put in the effort for this switch

I did a quick try (still not working) for using Bazel build for building this extension. It is more complicated even for only making a pbts command works in the build system. At first I thought it was at most just a bunch of genrules but it turns out it would be making npm repositories and I don't think I can finish them in a short time. (comparing to this pull request the first version is just done in under 10 minutes...)

Just saw #340 working on Bazel build, would move the discussion there.

cwahbong · 2024-10-23T13:45:50Z

Icon paths need to be fixed.

The esbuild.js is copied from VSCode extension generator template (github.com/microsoft/vscode-generator-code/tree/4812b7e/generators/app/templates/ext-command-ts/vscode-esbuild) then format fixed by prettier. This reduces the package size from ~5MB -> ~250KB.

cwahbong · 2024-10-25T12:10:21Z

I just made the corresponding changes in launch.json to support the debugging. I only validate that local breakpoint could be set in either "Launch Extension" and "Launch Extension Tests".

However I don't think I'm really understand how / why Debug Server works. It seems to be a debugging service for bazel build file instead of the plugin or typescript/javascript stuffs. I just made them into another binary, updating the referencedin package.json as well.

cwahbong · 2024-10-25T12:10:56Z

.vscode/launch.json

@@ -6,28 +6,29 @@
            "type": "extensionHost",
            "request": "launch",
            "runtimeExecutable": "${execPath}",
-            "args": ["--extensionDevelopmentPath=${workspaceRoot}"],
-            "stopOnEntry": false,


stopOnEntry is reported invalid entry in my vscode, thus removing it.

cwahbong · 2024-10-25T12:11:43Z

.vscode/launch.json

@@ -6,28 +6,29 @@
            "type": "extensionHost",
            "request": "launch",
            "runtimeExecutable": "${execPath}",
-            "args": ["--extensionDevelopmentPath=${workspaceRoot}"],


workspaceRoot is now deprecated

cwahbong · 2024-10-25T12:13:40Z

.vscodeignore

@@ -0,0 +1,10 @@
+# Ignore all files by default.


I found some examples and practices using .vscodeignore for white listing: so that we can invoke the vsce without changing the working directory or copying the included resources, and the layout for the resources should be the same no matter we minified or not.

cwahbong · 2024-10-25T12:16:47Z

Still looking into the "watch" flow. I also have no idea for how to test these flow automatically to prevent regression.
For now I assume all developers are daily using it and would be triggered once broken again.

cwahbong · 2024-10-25T13:10:34Z

package.json

@@ -483,6 +488,7 @@
        "eslint-plugin-jsdoc": "^48.2.2",
        "js-yaml": "^4.1.0",
        "mocha": "^10.4.0",
+        "npm-run-all": "^4.1.5",
        "prettier": "^3.2.5",


The usage of npm-run-all is referenced from https://github.com/microsoft/vscode-generator-code/blob/4812b7e/generators/app/templates/ext-command-ts/vscode-esbuild/package.json, but it has 1700+ dependent packages... I can revoke the modification for watch since I found it not really useful for vscode IDE...

That does seem like a lot of extra dependency weight (with the attendant impact on CI times and security alert noise) for something of marginal utility. Developers need to take explicit action to reload the extension anyway.

adam-azarchs · 2024-11-07T22:14:07Z

package.json

@@ -483,6 +488,7 @@
        "eslint-plugin-jsdoc": "^48.2.2",
        "js-yaml": "^4.1.0",
        "mocha": "^10.4.0",
+        "npm-run-all": "^4.1.5",
        "prettier": "^3.2.5",


That does seem like a lot of extra dependency weight (with the attendant impact on CI times and security alert noise) for something of marginal utility. Developers need to take explicit action to reload the extension anyway.

adam-azarchs · 2024-11-07T22:16:49Z

scripts/build.sh

-js-yaml syntaxes/bazelrc.tmLanguage.yaml > syntaxes/bazelrc.tmLanguage.json
-
-# Compile the rest of the project.
+# Compile the rest of the project for development flow like testing.
 tsc "$@" -p ./


Why do you still need tsc here? esbuild handles compilation. Running tsc remains useful for testing but not so much for build.

adam-azarchs · 2024-11-07T22:19:56Z

.vscodeignore

+!dist/**/*.js
+!icons/**/*.+(svg|license)
+!media/**/*
+!syntaxes/**/*.+(json|yaml|license)


yaml file shouldn't be included in the extension, only the form that's been compiled to json.

Suggested change

!syntaxes/**/*.+(json|yaml|license)

!syntaxes/**/*.+(json|license)

Do note that it was previously excluded.

cwahbong requested review from coeuvre, jfirebaugh, cameron-martin and vogelsgesang as code owners October 20, 2024 02:05

cwahbong changed the title ~~Bundle the extension using esbuild~~ chore: Bundle the extension using esbuild Oct 20, 2024

cwahbong force-pushed the esbuild branch from 221bf9e to 6c56b0c Compare October 20, 2024 02:12

vogelsgesang reviewed Oct 20, 2024

View reviewed changes

.vscodeignore Outdated Show resolved Hide resolved

package.json Outdated Show resolved Hide resolved

package.json Outdated Show resolved Hide resolved

package.json Show resolved Hide resolved

cwahbong force-pushed the esbuild branch from 7f5dd5e to ab1b271 Compare October 20, 2024 07:09

cwahbong requested a review from vogelsgesang October 20, 2024 07:10

vogelsgesang approved these changes Oct 20, 2024

View reviewed changes

scripts/package.sh Outdated Show resolved Hide resolved

cwahbong mentioned this pull request Oct 24, 2024

refactor(workspace-tree): Future proof icon path resolution #424

Merged

cwahbong added 2 commits October 25, 2024 12:06

Move the prepublish steps into package.sh

1cb2804

cwahbong force-pushed the esbuild branch 2 times, most recently from d336fe7 to d685dbb Compare October 25, 2024 12:01

cwahbong commented Oct 25, 2024

View reviewed changes

Fix ups for debugging flow

7ec3b91

cwahbong force-pushed the esbuild branch from d685dbb to 7ec3b91 Compare October 25, 2024 13:08

cwahbong commented Oct 25, 2024

View reviewed changes

cwahbong requested a review from vogelsgesang October 25, 2024 13:14

adam-azarchs reviewed Nov 7, 2024

View reviewed changes

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

chore: Bundle the extension using esbuild #421

chore: Bundle the extension using esbuild #421

cwahbong commented Oct 20, 2024 •

edited

Loading

vogelsgesang left a comment

vogelsgesang left a comment

cwahbong commented Oct 20, 2024 •

edited

Loading

cwahbong commented Oct 23, 2024

cwahbong commented Oct 25, 2024

cwahbong Oct 25, 2024

cwahbong Oct 25, 2024

cwahbong Oct 25, 2024

cwahbong commented Oct 25, 2024

cwahbong Oct 25, 2024 •

edited

Loading

adam-azarchs Nov 7, 2024

adam-azarchs Nov 7, 2024

adam-azarchs Nov 7, 2024

adam-azarchs Nov 7, 2024

	!syntaxes/*/.+(json\|yaml\|license)
	!syntaxes/*/.+(json\|license)

chore: Bundle the extension using esbuild #421

Are you sure you want to change the base?

chore: Bundle the extension using esbuild #421

Conversation

cwahbong commented Oct 20, 2024 • edited Loading

vogelsgesang left a comment

Choose a reason for hiding this comment

vogelsgesang left a comment

Choose a reason for hiding this comment

cwahbong commented Oct 20, 2024 • edited Loading

cwahbong commented Oct 23, 2024

cwahbong commented Oct 25, 2024

cwahbong Oct 25, 2024

Choose a reason for hiding this comment

cwahbong Oct 25, 2024

Choose a reason for hiding this comment

cwahbong Oct 25, 2024

Choose a reason for hiding this comment

cwahbong commented Oct 25, 2024

cwahbong Oct 25, 2024 • edited Loading

Choose a reason for hiding this comment

adam-azarchs Nov 7, 2024

Choose a reason for hiding this comment

adam-azarchs Nov 7, 2024

Choose a reason for hiding this comment

adam-azarchs Nov 7, 2024

Choose a reason for hiding this comment

adam-azarchs Nov 7, 2024

Choose a reason for hiding this comment

cwahbong commented Oct 20, 2024 •

edited

Loading

cwahbong commented Oct 20, 2024 •

edited

Loading

cwahbong Oct 25, 2024 •

edited

Loading