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

provide ability computed macro as a named export #70

Closed
miguelcobain opened this issue Nov 22, 2017 · 3 comments
Closed

provide ability computed macro as a named export #70

miguelcobain opened this issue Nov 22, 2017 · 3 comments

Comments

@miguelcobain
Copy link
Contributor

miguelcobain commented Nov 22, 2017

Now that we have the new Ember javascript modules API I think it would make sense to export the ability computed macro as its own export.

We could then use it like:

import Controller from '@ember/controller';
import { ability } from 'ember-can';

export default Controller.extend({
  // looks up the "post" ability and sets the model as the controller's "content" property
  postAbility: ability('post', 'content')
});

Another problem the current API has is that it is incompatible with ember's modules API unless you do some aliasing because you now import computed using:

import { computed } from '@ember/object';

which collides with

import { computed } from 'ember-can';

What I am doing at the moment is:

import { computed } from '@ember/object';
import { computed as can } from 'ember-can';

// and then use it like
someAbility: can.ability('post'),
someComputedProperty: computed(function() {
  return value;
})

It wouldn't break existing codebases since we would still be exporting computed as we are now.
We would also update the README to use this new API.

Is anyone opposed to this change?

@elidupuis
Copy link

I just came across this addon and immediately noticed the conflict with computed from @ember/object. @miguelcobain's suggestion of import { ability } from 'ember-can'; seems like the idiomatic Ember approach 👌.

I noticed that @Exelord was recently added as a contributor to help maintain the project; any thoughts on this?

@Exelord
Copy link
Collaborator

Exelord commented Apr 4, 2018

Hey! Unfortunately, Im not added yet 😅But yes, we can definitely improve public API and deprecate the old ones.

import { ability } from 'ember-can'; this is a bad idea as it will collide with import { Ability } from 'ember-can'; I mean, not directly but accidentally. To be honest, in my opinion, this whole thing with computed ability should be deprecated as it doesn't look like a good pattern. In most cases, you should use a service for resolving abilities or template helpers. You can also use a normal Ember.computed and use CanService#build to achieve the same result. It will do less magic and be more explicit, eg:

import { computed } from '@ember/object';

can: Ember.inject.service(),

postModel: computed(function(){
  return 'my model';
}),

ability: computed('postModel', function() {
  return this.get('can').build('post', this.get('postModel'));
})

I would also consider renaming build into abilityFor

@Exelord
Copy link
Collaborator

Exelord commented Apr 5, 2018

computed.ability will be deprecated and removed in further releases. We can use can service api to recreate it if we need. Just one method abilityFor (old build)

PR: #79

@Exelord Exelord closed this as completed Apr 5, 2018
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