glint icon indicating copy to clipboard operation
glint copied to clipboard

Fix problem with transitive generics in Signatures

Open gossi opened this issue 2 years ago • 3 comments

Potential fix to #610

I was applying the fix as part of this PR locally on my PR tildeio/ember-element-helper#107 and see if in my type-test-component:

import Component from '@glimmer/component';
import type { ElementSignature, ElementFromTagName } from 'ember-element-helper';

interface ElementReceiverSignature<T extends string = 'article'> {
  Element: ElementFromTagName<T>;
  Args: {
    tag: ElementSignature<T>['Return'];
  };
  Blocks: {
    default: [];
  }
}

export default class ElementReceiver<T extends string> extends Component<ElementReceiverSignature<T>> {}
{{#let @tag as |Tag|}}
  {{!@glint-ignore}}
  <Tag id="content" ...attributes>{{yield}}</Tag>
{{/let}}

I was able to remove the line {{!@glint-ignore}} and if the type linting still passes, which is the case.

So, yes this is a potential fix, but I share the same concerns as @dfreeman this might come with side-effects.

So I'll make it a draft PR.

gossi avatar Aug 17 '23 19:08 gossi

Thanks for opening this! The fact that CI is ✅ is a great initial sign – one of us will try to review in detail shortly.

chriskrycho avatar Aug 17 '23 20:08 chriskrycho

This should include, at a minimum, a new test case for the scenario this is intended to fix. Otherwise there's nothing to prevent us from flipping this back in the future and causing the same problem again

(I'd also recommend coming up with a more specific title for this PR, since that's what will go in the release notes—we clearly already "allow generics in signatures" in the general case, since many built-ins like let and each require that 😉)

dfreeman avatar Aug 21 '23 12:08 dfreeman

I added some tests. I copied over all the relevant types from (element) helper as well as a component receiving it. Feels a lot, but makes it accurate. If this can be less types, please let me know.

I also left comments where I got stuck. I think, I need to test broken/invalid types, in a environment with correct types?

gossi avatar Sep 09 '23 15:09 gossi