Add engines to Works With section #1058
Conversation
|
@jthomasmock @filiptronicek What do you think? |
| const displayName = getTargetPlatformDisplayName(targetPlatform); | ||
| return displayName ? <span key={targetPlatform}>{index > 0 ? ', ' : ''}{displayName}</span> : null; | ||
| }); | ||
| const renderWorksWithEngines = (engines?: Record<string, string>): ReactNode => { |
There was a problem hiding this comment.
I'd be great to make this use useCallback so that we memoize the return value without having to re-render every time. The same goes for renderWorksWithTargetPlatformsList as well.
There was a problem hiding this comment.
If we do, we should make the functions depend on extension.engines and extension.downloads respectively by removing the arguments and using extension there and always reference extension from the component itself.
There was a problem hiding this comment.
Sounds good. I'll add memoization.
| @@ -91,7 +91,7 @@ export interface Extension { | |||
| description?: string; | |||
|
|
|||
| // key: engine, value: version constraint | |||
| engines?: string[]; | |||
| engines?: Record<string, string>; | |||
There was a problem hiding this comment.
looks like this is nothing new, just aligning with reality 👍
| .map((engine) => ({ | ||
| key: engine, | ||
| name: getEngineDisplayName(engine), | ||
| version: engines[engine] |
There was a problem hiding this comment.
I wonder how much free rein extensions have here. Do we have any limits on what users can put into the engine version?
I don't think it's necessary to parse these values per se, but we could have some validation on the backend if we don't already.
There was a problem hiding this comment.
The vscode version is required, but a publisher can add other engines too. From https://code.visualstudio.com/api/references/extension-manifest:
Looks great to me! |
amvanbaren
force-pushed
the
webui-add-engine
branch
from
48a686d to
a0d16ab
Compare
amvanbaren
force-pushed
the
webui-add-engine
branch
from
a0d16ab to
999a978
Compare
Add engines to Works With section. Related issue: #1042
Some examples:
