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

Remove Record-based constraints on signature members #155

Merged
merged 3 commits into from
Apr 15, 2021

Conversation

dfreeman
Copy link
Member

Currently, people who have class MyComponent extends Component<MyArgs> and are adopting Glint are naturally inclined to change that to Component<{ Args: MyArgs }>. Unfortunately, if the MyArgs type is declared as an interface, that change will fail with a confusing error:

Type '{ Args: MyArgs; }' does not satisfy the constraint 'ComponentSignature'.
  Types of property 'Args' are incompatible.
    Type 'MyArgs' is not assignable to type 'Record<string, unknown>'.
      Index signature is missing in type 'MyArgs'.(2344)

The underlying reason for this is that TS will infer an index signature for type aliases, but is unwilling to do so for interfaces because that's more unsafe due to the potential for declaration merging. Needing to migrate interfaces to type declarations is an annoying adoption papercut, particularly given that it's not apparent from the error message that that's a viable fix.

In practice this constraint doesn't provide a whole lot of value over using object instead, particularly relative to the number of folks who've already run into this index signature issue. We may consider some type-aware linting in the future, but for now this should remove some real pain for people working to adopt Glint.

@dfreeman dfreeman added the bug Something isn't working label Apr 15, 2021
@chriskrycho
Copy link
Member

If only TS would actually let us describe these semantics usefully. 😩

@dfreeman dfreeman merged commit cf2e258 into master Apr 15, 2021
@dfreeman dfreeman deleted the remove-index-constraints branch April 15, 2021 16:00
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants