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

Introduce bundling with esbuild #283

Merged
merged 2 commits into from
Apr 24, 2023

Conversation

afonsonf
Copy link
Contributor

@afonsonf afonsonf commented Apr 23, 2023

Hi, in this PR I'm introduce bundling of the extension using esbuild (https://esbuild.github.io/). Documentation on bundling vscode extensions: https://code.visualstudio.com/api/working-with-extensions/bundling-extension.

I'm introducing bundling to enable the refactoring of the webui component using the webui-toolkit (#261).

Changes

  • Introduce bundling with esbuild
    • Entire extension code packaged to a single js file
    • Path reference to tools files changed as the js file location changed
  • Update .vscodeignore to be more clear what is packaged inside the extension
  • Update linter and cleanups (updated to the version used in webui-toolkit-samples)
    • Update the import of the moment module because of linter warnings
    • Remove unused variables and fix line too long

References

@afonsonf afonsonf requested a review from lemmy April 23, 2023 22:11
@afonsonf afonsonf changed the title change build to use esbuild Introduce bundling with esbuild Apr 23, 2023
@afonsonf afonsonf force-pushed the add-build-using-esbuild branch 2 times, most recently from ae871c0 to 88d9930 Compare April 23, 2023 22:31
@lemmy
Copy link
Member

lemmy commented Apr 23, 2023

@afonsonf, I just came across esbuild and am unable to provide much of a review. However, upon examining the diff, I suggest dividing the changes related to esbuild integration, linter changes, and vscode.dev support into distinct commits. This will help with when bisecting potential regressions in the future. Speaking of vscode.dev support, syntax highlighting will be a big improvement. :-) Can you make a note why "[I] was not able to make it work in codespaces"?

Copy link
Member

@lemmy lemmy left a comment

Choose a reason for hiding this comment

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

LGTM with minor suggestions at #283 (comment)

@afonsonf
Copy link
Contributor Author

afonsonf commented Apr 24, 2023

@afonsonf, I just came across esbuild and am unable to provide much of a review. However, upon examining the diff, I suggest dividing the changes related to esbuild integration, linter changes, and vscode.dev support into distinct commits. This will help with when bisecting potential regressions in the future. Speaking of vscode.dev support, syntax highlighting will be a big improvement. :-) Can you make a note why "[I] was not able to make it work in codespaces"?

Thanks for the suggestion, I will separate the changes into different commits

From what I saw, to enable the usage of the extension in vscode.dev was just setting this field in the package.json:
https://github.com/afonsonf/vscode-tlaplus/blob/add-build-using-esbuild/package.json#L41

However there is the error pop up when starting it, because it tries to load the entire extension, it would be nice to configure it to only load the highligting and the supported components:
image

For testing the extension in "web" mode this is the documentation:
(https://code.visualstudio.com/api/extension-guides/web-extensions#test-your-web-extension)

The first option did not work because the vscode debugging instance that started did not had the extension enabled or did not work, could not find why.

The errors I got in codespaces trying to use the second option where 500 erros when opening the vscode.dev instance started with @vscode/test-web. Tried changing some options ('--host' and codespace port configurations) but could not get it to work.

Locally (from a devcontainer environment), the second and thrid option worked, however the third one required more setup (I'll check if the instructions for the third one, sideloading, work from the codespace environment, and if it is the case I'll document it).

@lemmy lemmy added the enhancement New feature or request label Apr 24, 2023
@afonsonf
Copy link
Contributor Author

However there is the error pop up when starting it, because it tries to load the entire extension, it would be nice to configure it to only load the highligting and the supported components

I think this can be solved by introducing a new ts file as the browser extension

Signed-off-by: Afonso Fernandes <21228942+afonsonf@users.noreply.github.com>
Signed-off-by: Afonso Fernandes <21228942+afonsonf@users.noreply.github.com>
@afonsonf
Copy link
Contributor Author

Divided the changes in two commits and removed the changes related to vscode.dev because I will improve it and create a new PR with those changes

@lemmy
Copy link
Member

lemmy commented Apr 24, 2023

Feel free to push whenever you are ready. :-)

@afonsonf afonsonf merged commit 6eebff8 into tlaplus:master Apr 24, 2023
@lemmy
Copy link
Member

lemmy commented Apr 25, 2023

The extension fails to activate the first time when debugging the extension in a second browser window on Codespaces:

Activating extension 'alygin.vscode-tlaplus' failed: Cannot find module '/workspaces/vscode-tlaplus/out/main.js'
Require stack:
- /vscode/bin/linux-x64/704ed70d4fd1c6bd6342c436f1ede30d1cff4710/out/vs/loader.js
- /vscode/bin/linux-x64/704ed70d4fd1c6bd6342c436f1ede30d1cff4710/out/bootstrap-amd.js
- /vscode/bin/linux-x64/704ed70d4fd1c6bd6342c436f1ede30d1cff4710/out/bootstrap-fork.js.

Relaunching the debug session fixed it.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Development

Successfully merging this pull request may close these issues.

None yet

2 participants