Conversation
|
Oh, I should've looked around at first. I found there is also #107 which adds glint support. I took note and add the registry (as I forgot in the first place). Now, we have registry + export from index :) |
|
Can you make this pr only about the glint types? Upgrading embroider requires that we have a correct dep graph, and yarn doesn't provide that (nor is it capable) |
NullVoxPopuli
left a comment
NullVoxPopuli
left a comment
There was a problem hiding this comment.
This is looking amazing!!!!! thank you!!!!
NullVoxPopuli
left a comment
NullVoxPopuli
left a comment
There was a problem hiding this comment.
looks good to me, thanks for working on this!
There was a problem hiding this comment.
oh but wait -- we need to run a typescript matrix in Ci, to assure we have TS compatibility.
I usually do TS 4.8, 4.9, 5.0, and 5.1 when running glint on the test-app
here is an example: https://github.com/NullVoxPopuli/ember-resources/blob/main/.github/workflows/ci.yml#L50-L76
| interface ElementReceiverSignature<T extends string> { | ||
| Element: ElementFromTagName<T>; | ||
| Args: { | ||
| tag: ElementSignature<T>['Return']; | ||
| }; | ||
| Blocks: { | ||
| default: []; | ||
| } | ||
| } |
There was a problem hiding this comment.
This is my test type, but they appear not to work perfectly. Here is the glint error message:
app/components/element-receiver.hbs:2:3 - error TS2345: Argument of type 'NonNullable<ElementFromTagName<T>> extends never ? unknown : ElementFromTagName<T>' is not assignable to parameter of type 'Element'.
Type 'unknown' is not assignable to type 'Element'.
2 <Tag id="content" ...attributes>{{yield}}</Tag>
~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
app/components/element-receiver.hbs:2:21 - error TS2345: Argument of type 'NonNullable<ElementFromTagName<T>> extends never ? unknown : ElementFromTagName<T>' is not assignable to parameter of type 'Element'.
Type 'unknown' is not assignable to type 'Element'.
2 <Tag id="content" ...attributes>{{yield}}</Tag>
|
|
||
| > **Note:** Glint itself is still under active development, and as such breaking changes might occur. | ||
| > Therefore, Glint support by this addon is also considered experimental, and not covered by our SemVer contract! | ||
|
|
There was a problem hiding this comment.
Types docs start here. This is giving me problems. See below: https://github.com/tildeio/ember-element-helper/pull/107/files#r1286107261
|
Typechecks for TS 4.8+ with Glint. Had to use |
NullVoxPopuli
left a comment
NullVoxPopuli
left a comment
There was a problem hiding this comment.
Gonna try to get a review from the TS folks as well -- since the impact of this library is so wide
|
The problem is more likely to sit in upstream |
2 participants
This PR:
... turns out, the CI wasn't running for a while and this at least has some problems locally - it might be, the current
mainbranch is broken, too.