-
-
Notifications
You must be signed in to change notification settings - Fork 204
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
(feat) ComponentEvents interface #459
(feat) ComponentEvents interface #459
Conversation
This adds the possibility to use a reserved interface name `ComponentEvents` and define all possible events within it. Also adds autocompletion for these events. Also disables autocompletions from HTMLPlugin on component tags. sveltejs#424 sveltejs#304
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Mostly great, only an issue about string literal or computed property name needs more works
packages/language-server/src/plugins/typescript/features/CompletionProvider.ts
Outdated
Show resolved
Hide resolved
}); | ||
}); | ||
|
||
return map; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The name here is possibly a string literal or a computed property. i.e.
interface ComponentEvents {
'kebab-case': CustomEvent<number>
[SOME_CONSTANT]: CustomEvent<number>
}
getText
in either situation would be wrong. if it's a string literal the text can be extracted from member.name.text
.
About the computed property, from my testing, computed event handler like <Component on:[SOME_CONSTANT] /> doesn't work. Maybe we could make it works here but filter out in the completion. Or simply doesn't support it as it doesn't make much sense when you can't use it in markup.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Good catch! We should definitely support string literals. About the constant, we could go to the root and check if it's defined there and get the value from there - so we at least support it a little. I will also add docs about this.
…definition is given Enhanced tests to check other aspects of the svelte2tsx output
assert.equal(code, expectedOutput); | ||
assert.equal(output.code, expectedOutput); | ||
if (expecterOtherOutput) { | ||
expecterOtherOutput(output); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I enhanced the test suite so you can test other outputs of svelte2tsx
, see component-events-interface-constant/expected.js
for an example.
); | ||
error.start = toLineColumn(prop.getStart()); | ||
error.end = toLineColumn(prop.getEnd()); | ||
throw error; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This throws if the interface definition does not match our expextations. I hope this makes it clear to the user what is possible and what is not.
Thanks for the feedback! I added the requested changes. Let me know if it's all good now for you. |
can someone please provide on how to define type defs to include events too ? |
This is not official yet until this RFC is finalized and merged. You can type |
This adds the possibility to use a reserved interface name
ComponentEvents
and define all possible events within it.Also adds autocompletion for these events.
Also disables autocompletions from HTMLPlugin on component tags.
NO support for generics.
#424 #304 #442
TODO: