-
-
Notifications
You must be signed in to change notification settings - Fork 408
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
[Ember Data] Improve findHasMany/findBelongsTo decision making #387
Conversation
The approach here to resolving #356 is predicated on resolving several current implementation details, and would require leaking parts of private APIs we are seeking to eliminate. Furthermore, the approach entangles the concerns of the For these reasons, I think we should refactor this to pursue an approach based on unifying these requests at the Here's a quick overview of the primitives in play and their current responsibilities:
Ideally in the future the E.g. in the future a query for data (which may be a query for a relationship) would have the following flow, in which the cache is one of many potential sources and the adapter is responsible for assembling the list of resources to give back to the store that fulfill the query for data:
Here's what the API surface area for a request object that might make this simple could look like: interface RelationshipRequest {
// derived from previous API responses
links: Object|null;
meta: Object|null;
data: Object|Array|null;
// derived from local schema info
definition: Object|null;
parentRecordData: RecordData;
// request specific
options: Object|null;
// the state we are in, derived from the nature and result of various previous requests
relationshipState: RelationshipState;
} And here is what this might look like from an API perspective: // internal method when you call `record.get('relationshipName')
// or record.belongsTo('relationshipName').re/load(options)`
Relationship.getData(options): Array<Record>
=> RecordData.findBelongsTo(relationshipRequest): Collection (internal primitive)
=> Store.findBelongsTo(relationshipRequest): Collection (internal primitive)
=> Adapter.findBelongsTo(relationshipRequest): JsonApiDocument |
@mydea for the thing I am proposing above, yes, this means that coalescing would need to be an adapter level concern for this to continue working with |
So how would you propose that we proceed here? I generally agree with everything you wrote - these things make a lot of sense to me. Still, I feel like it would be great to have a way to do this in the meanwhile - as I guess all of these changes will be a certain time off, and it would be really great to have a way to handle this as soon as possible. Maybe, as a compromise, we could follow one of the alternatives I lined out in the meanwhile, splitting up |
Ideas, feel free to add to list or claim: - [ ] ```I've been getting a lot of questions about how tree-shaking is coming along. I would be willing to train anyone that wants to help on what's already done and what still needs to be done. Disclaimer: It's a lot of work! #emberjs #EmberFest``` https://twitter.com/kellyselden/status/1050717338595745792 - [ ] Include summary from pixelhandler (and cc him) #issue-triage team: https://discordapp.com/channels/480462759797063690/480523776845414412/500377829452546058 - [x] From jenweber on #support-ember-times: Another times topic: emberjs/ember.js#16910 I would summarize as, "ember.js has an addition to the test suite to help with API documentation! In the past, you might have noticed that a small number of methods had missing docs. When code gets moved around in ember.js, such as putting functions in their own modules, it's easy to make mistakes that impact the documentation parser. This test builds the docs and checks them for unintentional changes. Thanks <link to ed> for the test and to everyone who helped fix missing docs over the years." Todd Jordan did the majority of the fixes when things got dropped, I think. Hard to say for sure 🔏 @kennethlarsen - [x] emberjs/rfcs#384 (:lock: @jessica-jordan) - [x] emberjs/rfcs#386 (:lock: @jessica-jordan) - [ ] emberjs/rfcs#387 (:lock: @jessica-jordan) - [x] emberjs/rfcs#388 (:lock: @Alonski) - [ ] emberjs/rfcs#389 - [x] EmberConf brainstorming session date, if confirmed (:lock: @amyrlam) - [ ] Hacktoberfest roundup? - [ ] Check https://github.com/jessica-jordan/whats-new-in-emberland - [x] [ember-self-focused](https://engineering.linkedin.com/blog/2018/10/making-ember-applications--ui-transitions-screen-reader-friendly) (🔒 @chrisrng ) - [ ] ... Ideas we are waiting on: - [ ] EmberCamp talks, deep dive? http://embercamp.com/ and https://github.com/ember-chicago/ember-camp-2018-notes ... Maybe we wait for the talk videos to be published? Keep an eye on #ember-camp in Discord. not out yet - [ ] GraphQL with Ember? https://emberfest.eu/schedule/#rocky-neurock Or maybe we can reach out to them for a post-EmberFest writeup? See also the blog post: https://medium.com/kloeckner-i/ember-and-graphql-8aa15f7a2554
Some discussion around this went on in a recent ember-data team meeting, @runspired do you happen to recall the specifics? |
Ideas, feel free to add to list or claim! - [ ] I've been getting a lot of questions about how tree-shaking is coming along. I would be willing to train anyone that wants to help on what's already done and what still needs to be done. Disclaimer: It's a lot of work! https://twitter.com/kellyselden/status/1050717338595745792 - [x] emberjs/rfcs#387 it seems this was covered in [last week's edition](https://www.emberjs.com/blog/2018/10/19/the-ember-times-issue-69.html#toc_a-href-https-github-com-emberjs-rfcs-pull-387-using-relationships-links-or-not-your-call-a) - [ ] emberjs/rfcs#389 - [x] emberjs/rfcs#391 (:lock: @jessica-jordan) - [x] emberjs/rfcs#392 (:lock: @jessica-jordan) - [ ] Hacktoberfest roundup? - [ ] Check https://github.com/jessica-jordan/whats-new-in-emberland (roundup of all the PRs in emberjs and ember-learn repos) - [x] GraphQL with Ember? https://emberfest.eu/schedule/#rocky-neurock Or maybe we can reach out to them for a post-EmberFest writeup? See also the blog post: https://medium.com/kloeckner-i/ember-and-graphql-8aa15f7a2554 cc @simonihmig (:lock: @amyrlam) - [ ] #30DaysOfEmber https://twitter.com/PoslinskiNet/status/1054446639719608320 - [x] Following the spirit of building shared solutions, ember-i18n is now deprecated in favor of ember-intl (https://github.com/ember-intl/ember-intl …)! Now we have a blessed way to internationalize Ember apps! Check the codemod we built @DockYard to help with the migration https://twitter.com/MiguelCamba/status/1054699605865177089 (🔒 @chrisrng ) - [x] Ember 3.5 released! (🔏 @kennethlarsen) Ideas we are waiting on: - [ ] EmberCamp talks, deep dive? http://embercamp.com/ and https://github.com/ember-chicago/ember-camp-2018-notes ... Maybe we wait for the talk videos to be published? Keep an eye on #ember-camp in Discord. - [ ] EmberFest talks, deep dive? Keep an eye on #ember-fest in Discord. - [ ] Include summary from pixelhandler (get input from him), see #issue-triage in Discord (check toward end of October)
After further discussion OOB with @mydea we're going to accomplish this a different way. A new approach is being drafted to move coalescing into the adapter, which will support multiple efforts to disentangle our primitives in addition to making it easier to accomplish the original intent of this RFC which is to make all relationship requests use the same adapter entry points. |
@runspired is the new approach being tracked somewhere as of yet? |
Rendered
This is based on #356 & emberjs/data#5574 (comment).