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

Colocation does not work with anonymous classes and class decorators #583

Open
boris-petrov opened this issue Jul 8, 2020 · 9 comments
Open

Comments

@boris-petrov
Copy link

Moved from typed-ember/ember-cli-typescript#1166 . There is a reproduction repo there.

cc @chriskrycho

Quote from Chris in Discord:

I think it'll just need to tweak the transform so that if it's not a named class (BUT NAME YOUR CLASSES PEOPLE) it'll compile to

import Component, { setComponentTemplate } from '@ember/component';
import template from './component.hbs';

export default setComponentTemplate(template, class extends Component {});
@rwjblue rwjblue transferred this issue from ember-cli/ember-cli Jul 13, 2020
@rwjblue
Copy link
Member

rwjblue commented Jul 13, 2020

Transferred to ember-cli/ember-cli-htmlbars.

@rwjblue
Copy link
Member

rwjblue commented Jul 13, 2020

In general, I would think this already works properly. We have an explicit test in the babel plugin for this input:

it('sets the template for anonymous class default exports', function () {
let { code } = babel.transformSync(
stripIndent`
import Component from 'somewhere';
const __COLOCATED_TEMPLATE__ = 'ok';
export default class extends Component {}
`,
{ plugins: [ColocatedBabelPlugin] }
);
assert.strictEqual(
code,
stripIndent`
import Component from 'somewhere';
const __COLOCATED_TEMPLATE__ = 'ok';
export default Ember._setComponentTemplate(__COLOCATED_TEMPLATE__, class extends Component {});
`
);
});

If you could add another test to the broccoli plugin side (maybe there is an issue there and not in the babel plugin) like this:

it('works for component with template and class', async function () {
input.write({
'app-name-here': {
'router.js': '// stuff here',
components: {
'foo.hbs': `{{yield}}`,
'foo.js': stripIndent`
import Component from '@glimmer/component';
export default class FooComponent extends Component {}
`,
},
templates: {
'application.hbs': `{{outlet}}`,
},
},
});
let tree = new ColocatedTemplateCompiler(input.path());
output = createBuilder(tree);
await output.build();
assert.deepStrictEqual(output.read(), {
'app-name-here': {
'router.js': '// stuff here',
components: {
'foo.js': stripIndent`
import { hbs } from 'ember-cli-htmlbars';
const __COLOCATED_TEMPLATE__ = hbs("{{yield}}", {"contents":"{{yield}}","moduleName":"app-name-here/components/foo.hbs","parseOptions":{"srcName":"app-name-here/components/foo.hbs"}});
import Component from '@glimmer/component';
export default class FooComponent extends Component {}
`,
},
templates: {
'application.hbs': '{{outlet}}',
},
},
});
await output.build();
assert.deepStrictEqual(output.changes(), {}, 'NOOP update has no changes');
input.write({
'app-name-here': {
'router.js': '// other stuff here',
},
});
await output.build();
assert.deepStrictEqual(
output.changes(),
{ 'app-name-here/router.js': 'change' },
'has only related changes'
);
});

That uses export default class extends Component {} (anonymous class), then we could confirm if that is an issue or not.

@rwjblue
Copy link
Member

rwjblue commented Jul 13, 2020

@boris-petrov - A small reproduction would also be helpful here...

@boris-petrov
Copy link
Author

@rwjblue - sorry, I mentioned that there is a reproduction repo in the original issue I opened, I guess you've missed it. Here is the repo. Note that according to the other issue, ember-cli-typescript has nothing to do with the problem and it appears even when you remove ember-cli-typescript.

@rwjblue
Copy link
Member

rwjblue commented Jul 13, 2020

Ya, I was not really thinking it was an ember-cli-typescript issue, but I do think it is related to a combination of something else. For example, in that repro it looks like your input is leveraging a class decorator and we have #442 reporting issues in that scenario. We'd need to dig in and see if this is actually a duplicate of that issue...

@chriskrycho
Copy link
Contributor

chriskrycho commented Jul 13, 2020

Yeah, when @dfreeman poked at it, it looked like it was specifically to do with its being a decorated anonymous class—he could reproduce without ember-cli-typescript—but @boris-petrov's finding that it only showed up in his case when ember-cli-typescript was involved suggested to me that it is likely a matter of the ordering of the Babel plugins, since (until very soon now when we ship e-c-ts 4.0) e-c-ts impacts ordering of plugins.

Our ecosystem-level changes for ember-cli-babel and ember-cli-typescript should eliminate that as a cause of these kinds of things going forward, but that's likely (at least part of) the root issue there.

@rwjblue
Copy link
Member

rwjblue commented Mar 7, 2021

Is this the same as #442?

@boris-petrov
Copy link
Author

Well, in your previous comment you said that it could be... so it still could be. 😄

@rwjblue
Copy link
Member

rwjblue commented Mar 8, 2021

🤦‍♂️ - totally missed that while triaging issues 😭

But the good news is, old me and current me happen to agree on something...

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

No branches or pull requests

3 participants