Migrate extension webviews to Svelte 5#815
Open
nobody5050 wants to merge 25 commits intowpilibsuite:2027from
Open
Migrate extension webviews to Svelte 5#815nobody5050 wants to merge 25 commits intowpilibsuite:2027from
nobody5050 wants to merge 25 commits intowpilibsuite:2027from
Conversation
- Help - Project Creator - Project Importer - Dependency View
nobody5050
changed the title
sciencewhiz
reviewed
Feb 9, 2026
Member
|
Gold856
suggested changes
Feb 19, 2026
Member
There was a problem hiding this comment.
I may or may not review this more closely later, but just some broad details I'm noticing (and some of the recommendations may be wrong since I completed the Basic Svelte tutorial like 30 minutes ago):
- Almost all props seem to be optional, but that doesn't actually seem to be the case in practice; they're either defaulted in the child component or initialized with a value in the parent. In fact, several type definitions have their optional declaration erased because of that. How many props can we change to not be optional?
- Speaking of props, we're passing an insane number of props to several child components. Can some of this be replaced with global state in .svelte.ts files? Global state is kinda icky, but these webviews are small, so globals aren't used in very far away places.
- Let's try to completely dump Webpack in this PR. I'm not interested in dealing with multiple build systems.
- Can we make the config files use ES modules?
- There's a lot of functions without JSDoc. Try to add some API docs to newly added shared functions/components.
- Can we reduce the main.ts boilerplate by inlining as much as possible?
| {/if} | ||
| </div> | ||
| </div> | ||
| {:else} |
Member
There was a problem hiding this comment.
That is a seriously impressive amount of indentation (8 indents!) There has to be a way to simplify that...
Author
There was a problem hiding this comment.
The alternative as best I can tell would need to make use of $derived spam and would be truly unreadable. At least this is readable. Open to suggestions for other ways to solve it.
Gold856
suggested changes
Feb 19, 2026
Gold856
suggested changes
Feb 19, 2026
Gold856
suggested changes
Apr 16, 2026
Comment on lines
+5
to
+19
| type DistHtmlOptions = { | ||
| webview: vscode.Webview; | ||
| extensionRoot: vscode.Uri; | ||
| distHtmlFileName: string; | ||
| extraCss?: vscode.Uri[]; | ||
| appAttributes?: Record<string, string>; | ||
| }; | ||
|
|
||
| type RewriteHtmlOptions = { | ||
| webview: vscode.Webview; | ||
| extensionRoot: vscode.Uri; | ||
| html: string; | ||
| extraCss?: vscode.Uri[]; | ||
| appAttributes?: Record<string, string>; | ||
| }; |
Member
There was a problem hiding this comment.
Unify these and make the non-common fields a separate parameter at the call site.
| } from './types'; | ||
|
|
||
| const vscode = acquireVsCodeApi(); | ||
| const t = createTranslator('projectcreator'); |
Member
There was a problem hiding this comment.
Ugh. The current translation infrastructure is riddled with tech debt. Keeping compat with a future l10n transition would mean all calls look like l10n.t.
- Add rollup configuration to .vscodeignore - Re-sort package.json - Better checks for workspace in DependencyView - Clean up unused code in various files - Update TypeScript configuration to target ES2021 (allows promise.any)
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
4 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
This PR converts the webviews to a frontend framework, namely Svelte 5. All of the webviews are now built with rollup and share a lot more infrastructure, so i18n and similar should be easier to maintain. They also have the benefit of sharing components so the different wizards are easier to keep in sync.
Closes #55
Status: