ember data section class syntax (#543) #553
Conversation
| lastName: DS.attr(), | ||
| export default class Person extends Model { | ||
| @attr() firstName; | ||
| @attr() lastName; | ||
|
|
||
| fullName: computed('firstName', 'lastName', function() { |
There was a problem hiding this comment.
This mix of syntaxes has problems. See #554.
There was a problem hiding this comment.
Yup, I think for now we can just make this a standard ES6 getter, and not a computed property
| @@ -204,22 +208,23 @@ Requests for `user-profile` would now target `/user_profile/1`. | |||
| Some APIs require HTTP headers, e.g. to provide an API key. Arbitrary | |||
| headers can be set as key/value pairs on the `JSONAPIAdapter`'s `headers` | |||
| object and Ember Data will send them along with each ajax request. | |||
| (Note that we set headers in `init()` because default property values | |||
| (Note that we set headers in `constructor()` because default property values | |||
There was a problem hiding this comment.
Wouldn't using native classes supersede this implementation quirk ? IIRC, this pattern is because of EmberObject.extend #566
ghost
mentioned this pull request
ghost
changed the title
| headers: computed('session.authToken', function() { | ||
| export default class ApplicationAdapter extends JSONAPIAdapter { | ||
| @service session; | ||
| @computed('session.authToken') |
There was a problem hiding this comment.
I think we can remove @computed here and just make this a tracked property
There was a problem hiding this comment.
Rather, session.authToken should be considered a tracked property, and this can just be a normal getter
| @@ -580,15 +595,17 @@ You would define your relationship like this: | |||
|
|
|||
| ```javascript {data-filename=app/serializers/post.js} | |||
| import DS from 'ember-data'; | |||
| const { EmbeddedRecordsMixin, JSONSerializer } = DS; | |||
There was a problem hiding this comment.
The EmbeddedRecordsMixin will be tricky, ideally we can avoid mixins altogether. We'll have to work with the Ember Data team to figure out what the future is here.
There was a problem hiding this comment.
indeed. I chatted briefly with @runspired in #dev-ember-data to get a quick port of the syntax. Same theme there.
| lastName: DS.attr(), | ||
| export default class Person extends Model { | ||
| @attr() firstName; | ||
| @attr() lastName; | ||
|
|
||
| fullName: computed('firstName', 'lastName', function() { |
There was a problem hiding this comment.
Yup, I think for now we can just make this a standard ES6 getter, and not a computed property
Yes. See also the blueprints we just merged that do destructure. |
|
Thank you @efx for your work and sorting out some of these open questions! |
ghost
deleted the
* WIP class syntax * fix relationships snippets * mostly convert adapters + serializers * update to runspired's explanation of double extend * update other examples, fix typos * address comments to clean up examples * unify destructuring style
Current progress addressing #543.
See background discussion in the dev-ember-learning channel.
open questions / related issues
standardize decorators and remove usage of ember-decoratorsconst { Model, attr } = DS;to then use@attrfor model attributes. I think this syntax is clearer, but it is slightly inconsistent with example ember-data snippets. (e.g. extendingDS.JSONSerializer). Should we use destructuring anywhere we import fromDS?