Skip to content

[OCTANE] migrate a few other ember data examples#603

Merged
mixonic merged 3 commits intooctanefrom
unknown repository
Mar 18, 2019
Merged

[OCTANE] migrate a few other ember data examples#603
mixonic merged 3 commits intooctanefrom
unknown repository

Conversation

@ghost
Copy link

@ghost ghost commented Mar 14, 2019

Finish #543
@acorncom this should wrap up the few strictly ember data sections I had recalled. #594 covers the testing section that had some ember data examples as well.

@ghost ghost changed the title migrate a few other ember data examples [OCTANE] migrate a few other ember data examples Mar 14, 2019
Copy link
Contributor

@mike-north mike-north left a comment

Choose a reason for hiding this comment

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

Everything else looks good!


export default class RentalModel extends Model {
@attr title;
@attr owner;
Copy link
Contributor

@mike-north mike-north Mar 17, 2019

Choose a reason for hiding this comment

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

Ember-data decorators were mentioned but not listed in the API changes for emberjs/rfcs#408. I pinged @igorT to confirm (he's investigating) that this is (a) supported and (b) something we want to steer people towards.

cc: @pzuraq

Copy link
Author

Choose a reason for hiding this comment

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

cc: @mike-north thanks for fact checking this. I had opened another issue related to this. pzuraq gave some insight in this comment: #554 (comment)

Copy link

Choose a reason for hiding this comment

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

I double checked with @runspired and it's supported and people should use it

Copy link
Contributor

Choose a reason for hiding this comment

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

Yup, @mike-north this is actually not an @ember-decorators feature, it's a natural side effect of the fact that we support computed property macros in RFC 408. DS.attr is a computed property macro, as are most cool Ember helpers/features throughout the ecosystem, so they are all automatically decorators 😄

Copy link
Contributor

Choose a reason for hiding this comment

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

Oh, one thing that does need to change here though, I'm pretty sure you'll still have to do @attr() here, since these are functions that return decorators, and are not decorators themselves. Bit of a pain, but this will be the case until the ecosystem updates to support paren-less invocations.


Let's define the structure of a rental object using the same attributes for our rental that we [previously used](../model-hook/) in our hard-coded array of JavaScript objects -
_title_, _owner_, _city_, _category_, _image_, _bedrooms_ and _description_.
Define attributes by giving them the result of the function [`DS.attr()`](https://www.emberjs.com/api/ember-data/release/classes/DS/methods/attr?anchor=attr).
Copy link
Contributor

Choose a reason for hiding this comment

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

Can you update the prose to check for other references to the method DS.attr()?

Copy link
Author

Choose a reason for hiding this comment

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

@jenweber ah, so update it to attr()?

Copy link
Author

Choose a reason for hiding this comment

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

I updated each reference to attr as well as other references using DS.something

Copy link
Contributor

@mixonic mixonic left a comment

Choose a reason for hiding this comment

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

I've got hesitations regarding some of this, but my concerns can be addressed in future work. This looks great, thanks @efx

@mixonic mixonic merged commit b09b39e into ember-learn:octane Mar 18, 2019
@ghost ghost deleted the fix-543-1 branch March 18, 2019 14:46
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

6 participants