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

components are prefixed with this for complex structured apps #45

Closed
suchitadoshi1987 opened this issue Nov 5, 2019 · 11 comments · Fixed by #49
Closed

components are prefixed with this for complex structured apps #45

suchitadoshi1987 opened this issue Nov 5, 2019 · 11 comments · Fixed by #49

Comments

@suchitadoshi1987
Copy link

For an app that has multiple apps co-located or if there are components invoked from addons that reside outside of the project, the component gets prefixed with this when running the codemod.

@NullVoxPopuli
Copy link
Collaborator

Hi there! Thanks for the report!
Is there anyway you can provide some info about your project layout, where the component lives, etc?

@rwjblue
Copy link
Member

rwjblue commented Nov 5, 2019

@suchitadoshi1987 - Lets make a small reproduction repo (should use in-repo addons, ember-holy-futuristic-template-namespacing-batman, and components inside nested folders), should be a fun little exercise.

@suchitadoshi1987
Copy link
Author

@rwjblue @NullVoxPopuli Here is an example which has a couple of in-repo-addons as well as usage of ember-holy-futuristic-template-namespacing-batman syntax.
Here is the Repo:
https://github.com/lbdm44/overly-complex-ember-storefront
AND
npx ember-no-implicit-this-codemod http://localhost:4200 lib/item-description/addon/templates/components/item-detail.hbs is the codemod command

My hunch is that this is more of an issue with the missing support for the $ syntax than anything else.

@rwjblue
Copy link
Member

rwjblue commented Nov 15, 2019

Ya, makes sense. I think our module mapping needs to know what $ does in this context.

Seems like this is where we are building up a list of the "non-this" things:

function populateInvokeables() {
let components = [];
let helpers = [];
let telemetry = getTelemetry();
for (let name of Object.keys(telemetry)) {
let entry = telemetry[name];
switch (entry.type) {
case 'Component':
components.push(name);
break;
case 'Helper':
helpers.push(name);
break;
}
}
return [components, helpers];
}

It seems like it is pushing the component name directly there (perhaps still including the root path?), and we need to ensure $ usage is also accommodated...

@RobbieTheWagner
Copy link

I'm seeing this issue too. It's taking components like this: {{action-status-list and making them {{this.action-status-list. Perhaps we could ignore anything starting with {{ or {{#?

@NullVoxPopuli
Copy link
Collaborator

@rwwagner90 that may be a different issue.
couple questions for potential paths:

  • is action-status-list from an addon?
    • full addon or in-repo?
  • do you have telemetry data?
    • I've heard reports of telemetry data failing to be collected recently :(
  • which OS are you on?

@RobbieTheWagner
Copy link

@NullVoxPopuli I am on Mac, and that component is not from an addon. However, we do use pods for components, so it's possible it did not find those components for that reason.

@NullVoxPopuli
Copy link
Collaborator

Ah, yup. Pods would do it. We need to update telemetry helpers to support pods and co-location.

@RobbieTheWagner
Copy link

@NullVoxPopuli so pods should work now?

@NullVoxPopuli
Copy link
Collaborator

@rwwagner90 I don't think so. I think GitHub auto-closed this issue by mistake.
I mostly just don't know how the ember-holy-futuristic-template-namespacing-batman support would affect pods. But maybe I've forgotten too much of how things work. If you could give 0.8.0 a go, and confirm that this is still a big, that'd be fantastic!

@NullVoxPopuli NullVoxPopuli reopened this Dec 11, 2019
@Turbo87
Copy link
Contributor

Turbo87 commented Dec 15, 2019

pods support is tracked in #36 and the batman addon is supported now, so I'll go ahead and close this issue again :)

@Turbo87 Turbo87 closed this as completed Dec 15, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
5 participants