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

Unexpected behavior for WebviewMessageParticipant interface #5

Closed
msujew opened this issue Jul 6, 2022 · 2 comments
Closed

Unexpected behavior for WebviewMessageParticipant interface #5

msujew opened this issue Jul 6, 2022 · 2 comments
Assignees

Comments

@msujew
Copy link
Member

msujew commented Jul 6, 2022

Currently, using the WebviewMessageParticipant instead leads into a non-obvious pitfall:

export interface WebviewMessageParticipant {
type: 'webview'
/** Identifier of a specific webview instance. */
webviewId?: string
/** Webview panel type or webview view type. */
webviewType?: string
}

As both webviewId and webviewType are optional, I believed them both to be truly optional. However, at least one of them is needed. This fact is not reflected in the type definition. Instead, the type should probably look something like this:

export type WebviewMessageParticipant = WebviewIdMessageParticipant | WebviewTypeMessageParticipant;

export interface WebviewIdMessageParticipant {
    type: 'webview'
    /** Identifier of a specific webview instance. */
    webviewId: string
}

export interface WebviewTypeMessageParticipant {
    type: 'webview'
    /** Webview panel type or webview view type. */
    webviewType: string
}
@dhuebner
Copy link
Member

dhuebner commented Jul 7, 2022

@msujew
Yes, you are right it also what the docu comment says:

A webview must be identified either with an ID (webviewId) or a type (webviewType).

I like your proposal!

@dhuebner dhuebner self-assigned this Jul 7, 2022
@dhuebner
Copy link
Member

dhuebner commented Aug 4, 2022

Changed as proposed. Also added isWebviewIdMessageParticipant to check the WebviewMessageParticipant type
Thanks @msujew !

@dhuebner dhuebner closed this as completed Aug 4, 2022
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

No branches or pull requests

2 participants