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

Support TypeScript merging of export default declarations in template colocation #677

Merged
merged 3 commits into from
Mar 7, 2021

Conversation

dfreeman
Copy link
Contributor

@dfreeman dfreeman commented Mar 7, 2021

In TypeScript it's possible to merge together multiple interfaces by redeclaring them, and one occasional use for this is to declare that a class meets a certain interface in a way that isn't statically visible (such as the way @ember/components "implement" their args by splatting them on to the instance).

For a default export, though, this can result in multiple ExportDefaultDeclarations in one module:

import Component from '@ember/component';

type MyComponentArgs = { required: string; optional?: number };

export default interface MyComponent extends MyComponentArgs {}
export default class MyComponent extends Component {
  // ...
}

This is legal (since the TS plugin ultimately strips out the interface), but currently causes an error in the colocation plugin because we try to associate the template with the interface declaration rather than the class:

TypeError: unknown: 
  Property arguments[1] of CallExpression expected node to be of a type ["Expression","SpreadElement","JSXNamespacedName","ArgumentPlaceholder"] but instead got "TSInterfaceDeclaration"

This PR adds a check to ensure that the item we're attempting to associate the template to is either a class declaration or an expression so that we correctly skip the interface.

@rwjblue rwjblue added the bug label Mar 7, 2021
Copy link
Member

@rwjblue rwjblue left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I made one small suggestion, but overall this seems good.

lib/colocated-babel-plugin.js Outdated Show resolved Hide resolved
dfreeman and others added 2 commits March 7, 2021 18:27
Co-authored-by: Robert Jackson <me@rwjblue.com>
@rwjblue rwjblue changed the title Handle TS declaration merging in template colocation Support TypeScript merging of export default declarations in template colocation Mar 7, 2021
@rwjblue rwjblue merged commit f0efdd2 into ember-cli:master Mar 7, 2021
@rwjblue
Copy link
Member

rwjblue commented Mar 7, 2021

Released in https://github.com/ember-cli/ember-cli-htmlbars/releases/tag/v5.6.4

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants