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

[DOC release] Update documentation for store #3794

Merged
merged 1 commit into from
Sep 23, 2015
Merged

Conversation

e00dan
Copy link
Contributor

@e00dan e00dan commented Sep 22, 2015

This attempts to resolve #3687 and update other parts of store docs to mention JSONAPI move of ED. It's currently a massive WIP, but I'm open to all feedback.

* `id` - mandatory, unique record's key
* `type` - mandatory string which matches `model` name (can be singular or plural form)
* `attributes` - object which holds data for record attributes - `DS.attr`'s declared in model
* `relationships` - object which must contain any of the following:
Copy link
Contributor

Choose a reason for hiding this comment

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

Could you mention that links, data and meta are supposed to be under each relationships' respective key, to remove confusion.

@wecc
Copy link
Contributor

wecc commented Sep 23, 2015

Nice PR @kuzirashi, thanks! Left a couple of comments for minor tweaks.

@e00dan
Copy link
Contributor Author

e00dan commented Sep 23, 2015

Thanks for awesome feedback. What do you think now? @wecc

@@ -58,12 +58,12 @@ var dasherize = Ember.String.dasherize;
"data": [
{
"id": "3",
"type": "players"
"type": "player"
Copy link
Member

Choose a reason for hiding this comment

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

I believe the JSONAPISerializer expects types to be plural.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Store.push expects singular, JSON API serializer expects plural? I thought it is unified. Should I revert changes to json-api-serializer?

Copy link
Contributor

Choose a reason for hiding this comment

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

@bmac is correct. The internal Ember Data JSON API format differs a bit from the one used by JSONAPISerializer.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

This is confusing, at least to me. Shouldn't this be mentioned somewhere, somehow?

Copy link
Contributor

Choose a reason for hiding this comment

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

ED uses camelCased attributes and singular types internally, regardless of what adapter/serializer you're using.

When you're using the JSON API adapter/serializer we want users to be able to use the examples available on jsonapi.org and have it just work. Most users never have to care about the internal format since the serializer handles the work for them.

This is documented in the guides, http://guides.emberjs.com/v2.0.0/models/pushing-records-into-the-store/

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Thanks for explanation.

Copy link
Member

Choose a reason for hiding this comment

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

I think we should document this difference in some other places. Not everyone reads the guides.

@bmac
Copy link
Member

bmac commented Sep 23, 2015

This looks great @kuzirashi. Excited to see this merged.

@e00dan
Copy link
Contributor Author

e00dan commented Sep 23, 2015

@bmac, @wecc are we able to embed ember-twiddles via iframe in docs?

@e00dan
Copy link
Contributor Author

e00dan commented Sep 23, 2015

Changes applied.

@e00dan e00dan changed the title [WIP] Update documentation for store Update documentation for store Sep 23, 2015
@e00dan e00dan changed the title Update documentation for store [DOC release] Update documentation for store Sep 23, 2015
@bmac
Copy link
Member

bmac commented Sep 23, 2015

Its possible to embed an ember-twiddles via iframe in the docs by just inlining the html. However, they are discouraged because they are slow to load, are not very PR friendly and in general lead to some extra overhead in managing them which makes it harder to notice when iframe content is out of date.

@bmac
Copy link
Member

bmac commented Sep 23, 2015

I think this pr looks good. @kuzirashi do you mind squashing your commits into 1 and prepending the commit message with [DOC beta]?

@e00dan
Copy link
Contributor Author

e00dan commented Sep 23, 2015

Done. @bmac

bmac added a commit that referenced this pull request Sep 23, 2015
[DOC release] Update documentation for store
@bmac bmac merged commit ec0d4f1 into emberjs:master Sep 23, 2015
@wecc
Copy link
Contributor

wecc commented Sep 23, 2015

Great work @kuzirashi!

@bmac
Copy link
Member

bmac commented Sep 23, 2015

Thanks @kuzirashi.

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

Successfully merging this pull request may close these issues.

store.push API documentation outdated
3 participants