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

✨ NPM workspaces and Migrate to Vite for HMR in Webview Package #71

Merged
merged 14 commits into from
Oct 24, 2024

Conversation

ibolton336
Copy link
Member

@ibolton336 ibolton336 commented Oct 17, 2024

PR changes:

  • Replace Webpack with Vite for webview-ui. In development mode, the extension is able to consume the webview running on a local vite dev server which allows for HMR and a rapid development experience.
  • Adds npm workspaces implementation for dependency sharing (typescript) & better handling of build / dev scripts for development / production. Lifts all eslint & prettier configuration to project root.
    • Creates a new shared package that holds shared code between webview-ui and extension.
    • Can now run npm install && npm run dev from the project root to start both development servers (webpack for extension & vite for ui)
    • Vite enables HMR for webview development. Also removes need for complex plugin configuration - only plugin needed now is the vite react plugin
  • Adds a new debugger pre launch task to start dev servers. This allows us to start all development servers & open the debugger with f5.

Fixes #77

TODO:

  • Replace webpack copy plugin with something more robust
  • Address vscode extension state management
  • Improve webview messaging

@ibolton336 ibolton336 changed the title configure vite hmr ✨ Webpack -> Vite Oct 17, 2024
@ibolton336 ibolton336 marked this pull request as ready for review October 17, 2024 18:10
@ibolton336 ibolton336 force-pushed the HMR-config branch 2 times, most recently from d45b207 to 8011a56 Compare October 18, 2024 14:36
Copy link
Member

@sjd78 sjd78 left a comment

Choose a reason for hiding this comment

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

Does adopting vite require setting up webview-ui as a separate project under vscode? That is a bit concerning for knowing how to work in the project.

What does the future look like? The webview as it's own project and then any regular extension points go in the base vscode tree?

Would setting up a more typical multi-workspace project make more sense? Something like how tackle2-ui is setup? The root project has build related configs, then common is stuff shared between webview and "normal" extensions, then webview, then "normal" extensions?

Copy link
Member

@djzager djzager left a comment

Choose a reason for hiding this comment

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

I don't see anything technically wrong with this. With it we need to document how this changes the development workflow. Either the vscode README.md or contributing doc or some combination of the two.

vscode/README.md Outdated Show resolved Hide resolved
@ibolton336 ibolton336 force-pushed the HMR-config branch 2 times, most recently from fef8f5b to 6613a49 Compare October 18, 2024 16:31
@ibolton336
Copy link
Member Author

Does adopting vite require setting up webview-ui as a separate project under vscode? That is a bit concerning for knowing how to work in the project.

What does the future look like? The webview as it's own project and then any regular extension points go in the base vscode tree?

Would setting up a more typical multi-workspace project make more sense? Something like how tackle2-ui is setup? The root project has build related configs, then common is stuff shared between webview and "normal" extensions, then webview, then "normal" extensions?

The idea is here is based off the continue approach where they have a separate GUI project that remains separate from the extension code. Still working out what that looks like for us - will need to rethink CI with your input when the time is right.

@sjd78
Copy link
Member

sjd78 commented Oct 18, 2024

The idea is here is based off the continue approach where they have a separate GUI project that remains separate from the extension code. Still working out what that looks like for us - will need to rethink CI with your input when the time is right.

I understand what you're trying to do now. That is a great bit of info to put in the PR description

Should the webview/gui project then live outside of the vscode project tree? That way it is obvious that the webview is intended to be independent of the vscode work.

@ibolton336 ibolton336 force-pushed the HMR-config branch 3 times, most recently from c6fe38b to 6b6f2bb Compare October 18, 2024 21:06
@ibolton336 ibolton336 changed the title ✨ Webpack -> Vite ✨ Move Webview to Its Own Directory and Migrate to Vite for HMR in Webview Package Oct 18, 2024
@ibolton336 ibolton336 force-pushed the HMR-config branch 2 times, most recently from 1ca56c8 to 3c82043 Compare October 18, 2024 21:42
djzager
djzager previously approved these changes Oct 19, 2024
@ibolton336 ibolton336 changed the title ✨ Move Webview to Its Own Directory and Migrate to Vite for HMR in Webview Package ✨ NPM workspaces and Migrate to Vite for HMR in Webview Package Oct 22, 2024
@ibolton336 ibolton336 force-pushed the HMR-config branch 5 times, most recently from 88b03dc to 02388e5 Compare October 22, 2024 14:09
Signed-off-by: Ian Bolton <ibolton@redhat.com>
Signed-off-by: Ian Bolton <ibolton@redhat.com>
…ebpack

Signed-off-by: Ian Bolton <ibolton@redhat.com>
Signed-off-by: Ian Bolton <ibolton@redhat.com>

Signed-off-by: Ian Bolton <ibolton@redhat.com>
Signed-off-by: Ian Bolton <ibolton@redhat.com>
Signed-off-by: Ian Bolton <ibolton@redhat.com>
@ibolton336 ibolton336 force-pushed the HMR-config branch 4 times, most recently from 1adbcaf to 02cae1e Compare October 22, 2024 21:10
Signed-off-by: Ian Bolton <ibolton@redhat.com>
Signed-off-by: Ian Bolton <ibolton@redhat.com>
Copy link
Member

@sjd78 sjd78 left a comment

Choose a reason for hiding this comment

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

This looks very close to being able to open the repo root directory in vscode and then work on both the vscode/ extension and the webview/ component all at the same time.

Running things is kinda interesting given the target. So really I see a bunch of things going on:

  1. Hitting the run button in vscode to launch another vscode with the extension running (and the webview embedded in it)
  2. Doing npm run and having vscode with the extension running
  3. Doing a build to static files to be packaged
    I'll need to tinker with what is here to see which one of those things work etc. Working with running a command and then also hitting the run button is clunky but probably works ok for now. Feels like it could be streamlined.

Beyond that, it would be good to see the messages that get sent between the webview and the extension be well defined somewhere like "webview-api" or something. No better name comes to mind at the moment. Then the webview would grab an instance of that object/whatever and the actions would be all well defined. It is quite interesting to conceptualize the webview as an iframe in a browser that only has message posting access. I'm not sure that's actually the case here.

I will look again in the morning with coffee.

vscode/src/extension.ts Show resolved Hide resolved
vscode/src/client/analyzerClient.ts Outdated Show resolved Hide resolved
.gitignore Outdated Show resolved Hide resolved
@ibolton336
Copy link
Member Author

ibolton336 commented Oct 23, 2024

  • Hitting the run button in vscode to launch another vscode with the extension running (and the webview embedded in it)

This one was a head scratcher - couldn't get vscode debugger to open the launch.json unless I opened the nested vscode dir. Wanted to be able to launch the debugger from the project root, so had to create a new .vscode/launch.json in the project root.

Doing npm run and having vscode with the extension running
This should now word from the root of project with npm run dev. Updated the project root readme to reflect these dev experience changes.

Doing a build to static files to be packaged
I'll need to tinker with what is here to see which one of those things work etc. Working with running a command and then also hitting the run button is clunky but probably works ok for now. Feels like it could be streamlined.

This does work via webpack copying the built assets from vite build output into the out/webview directory. The copy plugin is configured to only run in prod mode.

Agree that the npm run dev command could be kicked off by the launch.json in some fashion. I will look into getting that working.

@ibolton336
Copy link
Member Author

Beyond that, it would be good to see the messages that get sent between the webview and the extension be well defined somewhere like "webview-api" or something. No better name comes to mind at the moment. Then the webview would grab an instance of that object/whatever and the actions would be all well defined. It is quite interesting to conceptualize the webview as an iframe in a browser that only has message posting access. I'm not sure that's actually the case here.

Interesting point(s) and I think that there should be issues created if they don't exist already to capture the messaging changes & the state handling changes.

Signed-off-by: Ian Bolton <ibolton@redhat.com>
Signed-off-by: Ian Bolton <ibolton@redhat.com>
Signed-off-by: Ian Bolton <ibolton@redhat.com>
djzager
djzager previously approved these changes Oct 24, 2024
Copy link
Member

@djzager djzager left a comment

Choose a reason for hiding this comment

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

lgtm

.nvmrc Outdated Show resolved Hide resolved
webview-ui/src/types.ts Outdated Show resolved Hide resolved
Signed-off-by: Ian Bolton <ibolton@redhat.com>
@ibolton336 ibolton336 merged commit 6527244 into konveyor:main Oct 24, 2024
13 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.

[vscode] dev mode flag so we have a reusable mechanism for faking/mocking out hard stuff
3 participants