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

Fix looking up abilities #176

Merged
merged 6 commits into from
Jul 2, 2024
Merged

Conversation

mkszepp
Copy link
Contributor

@mkszepp mkszepp commented Jul 2, 2024

This is a possible fix for our issue since v5 that abilities were not anymore found

README.md Outdated
@@ -30,6 +30,9 @@ Install this addon via ember-cli:
ember install ember-can
```

After install add `import 'ember-can/resolver';` inside your app.[js|ts]
Copy link
Collaborator

Choose a reason for hiding this comment

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

Is there not a way we can make this automatic?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yes over blueprint for new installations.... i will look to do it asap

Copy link
Contributor Author

Choose a reason for hiding this comment

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

For current installs we should add under changeloga that this change must be applied

Copy link
Collaborator

Choose a reason for hiding this comment

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

Sorry, I didn't mean automatically add this to files, I meant remove the need for it completely. Like using the initializer before or whatever.

Copy link
Collaborator

Choose a reason for hiding this comment

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

I'm getting No ability type found for foo for all my abilities in my app. Will this PR fix that? I definitely think we should have some kind of initializer that sets all this up automatically. I don't think initializers are deprecated or anything, so I am not sure why we removed it?

Copy link
Contributor Author

@mkszepp mkszepp Jul 2, 2024

Choose a reason for hiding this comment

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

@RobbieTheWagner yes this is exact the error which i have also got in our app... this PR solves the error. There was no test present which has tested this yet (with this PR we have one).

The initializer has solved the error in which they were run years ago... but they have exported always a empty initializer and added this code part outside of them. It was more/less a workaround to don't teach consumers that they need to add something inside the app.
At the end the previouse initializer looks for me more as a "hack".

I have already tried to readd it like before (inside src folder and also directly exported like blueprints) but it isn't working...

Copy link
Contributor Author

Choose a reason for hiding this comment

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

i have also asks glime in Discord, but glime tells me nothing helpful.

@NullVoxPopuli
Copy link

what's the downside of not doing this?

Does the app/abilities folder have to be renamed?

  • app/abilitys?

@RobbieTheWagner RobbieTheWagner changed the title Readd resolver, update ReadMe and add ability test Fix looking up abilities Jul 2, 2024
@RobbieTheWagner RobbieTheWagner merged commit 385f09a into minutebase:master Jul 2, 2024
13 checks passed
@mkszepp mkszepp deleted the readd-resolver branch September 17, 2024 18:20
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.

3 participants