Typescript JSDoc Part 1 - Add template parameters to mixins #5140
Conversation
|
Thanks for the PR! 🎉 We've deployed an automatic preview for this PR - you can see your changes here:
Note The build needs to finish before your changes are deployed. |
helpers/composeMixins.d.ts
Outdated
There was a problem hiding this comment.
| * @template {ReactiveElementClassType} S | ||
| * @param {S} superclass | ||
| */ | ||
| const InternalSkeletonGroupMixin = superclass => class extends SkeletonMixin(superclass) { |
There was a problem hiding this comment.
Using Internal mixin constant to avoid unnecessary whitespace changes.
| constructor() { | ||
| super(); | ||
| constructor(...args) { | ||
| super(...args); |
There was a problem hiding this comment.
The typescript compiler is picky with mixin constructor arguments, and won't type things correctly if ...args are missing.
awikkerink
force-pushed
the
awikkerink/inspiration-add-template-parameters-to-mixins
branch
from
a8cfdb2 to
1bda15f
Compare
awikkerink
force-pushed
the
awikkerink/inspiration-add-template-parameters-to-mixins
branch
3 times, most recently
from
36b0f26 to
967d71a
Compare
awikkerink
force-pushed
the
awikkerink/inspiration-add-template-parameters-to-mixins
branch
3 times, most recently
from
267f0f5 to
128e003
Compare
awikkerink
force-pushed
the
awikkerink/inspiration-add-template-parameters-to-mixins
branch
from
128e003 to
80789a2
Compare
awikkerink
changed the title
awikkerink
force-pushed
the
awikkerink/inspiration-add-template-parameters-to-mixins
branch
from
80789a2 to
d74be6c
Compare
* Maintaining a common.d.ts in this repo or elsewhere isn't desired
helpers/composeMixins.d.ts
Outdated
There was a problem hiding this comment.
@dlockhart would this be something you would want maintained in another repo?
There was a problem hiding this comment.
Is it possible for everything to be in another repo? Ideally a repo like core that isn't TS-aware shouldn't need to have any TS stuff in it.
There was a problem hiding this comment.
The jsdoc stuff being in another repo would be difficult to maintain and would make navigating to the component definitions more difficult.
|
I've converted these to Draft status so we don't get daily notifications about them while the overall TS strategy gets figured out. |
|
Hi @awikkerink , I noticed #5139 (comment) and was curious if Part 1 and 3 were also going to get culled under the same logic? |
No. Typing the mixins and exporting the components is useful for dependents. It also makes putting the HTMLTagElementMap in another repo possible. |
Perhaps I should rename the PRs then to make it more clear. My original intention was to add the tag maps, but this added too much complexity and not enough value. But I know the mixin typing will be useful. |
awikkerink
changed the title
This can be added to a separate repo.
This is the trickiest part to automate, but with #5139, it's possible to verify which mixins aren't typed correctly. Basically, the lint check will complain if the mixin doesn't return a type that extends HTMLElement, so an improperly typed mixin will fail the check.