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

Resource object name/class inconsistency #94

Closed
kenjiqq opened this issue Jul 17, 2014 · 7 comments
Closed

Resource object name/class inconsistency #94

kenjiqq opened this issue Jul 17, 2014 · 7 comments
Assignees
Milestone

Comments

@kenjiqq
Copy link

kenjiqq commented Jul 17, 2014

When you create a definition for a resource that provides the method object the created instances of this resource will be of a class with the same name as the name attribute in that resources definition instance.constructor.name === resourceDefinition.name.

But resources defined without the methods object will be of the class Object.
I feel this should be consistent across all instances of resources, and that they should all be of a class by the same name as the resource.

@kenjiqq kenjiqq changed the title Resource object name inconsistency Resource object name/class inconsistency Jul 17, 2014
@kentcdodds
Copy link
Contributor

I didn't realize that resources without a methods object had a class... If they do, what you suggest makes sense...

@kenjiqq
Copy link
Author

kenjiqq commented Jul 17, 2014

I might be mistaken there, will have to check maybe they didn't have a class. Though I still think it should be consistent, either a class or not

@jmdobry
Copy link
Member

jmdobry commented Jul 17, 2014

I agree, though I'll take it a step further. I conditionally wrap injected items with a class because some developers don't use the methods configuration wouldn't want to added overhead of every single item being wrapped by a constructor function. I'll add a configuration variable per resource definition that enables/disables wrapping items in the constructor. I think it should default to true.

@kenjiqq
Copy link
Author

kenjiqq commented Jul 17, 2014

Maybe add it to the global DS configuration as well to allow the default true to be changed to false, while supporting pr resource overrides?

@kentcdodds
Copy link
Contributor

I would argue it should default to false. Shouldn't make people pay with performance loss on things they don't know that they're getting...

@jmdobry
Copy link
Member

jmdobry commented Jul 17, 2014

Resource configurations inherit from the global configuration prototype, so if you set anything on DS.defaults it will propagate to the resources.

@kentcdodds And I think you're right, it should default to false.

When I have time in the next few days I will crank through a bunch of these issues, baby takes priority :)

@jmdobry jmdobry added this to the 0.10.1 milestone Jul 18, 2014
@jmdobry jmdobry self-assigned this Jul 18, 2014
@jmdobry
Copy link
Member

jmdobry commented Jul 20, 2014

Fixed as of 0.10.1.

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

No branches or pull requests

3 participants