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

Docs issue - What's the correct way to create and query one-to-many relationships with DataStore/AppSync? #2145

Closed
Jahans3 opened this issue Jul 23, 2020 · 11 comments
Labels
amplify/js Issues tied to JS

Comments

@Jahans3
Copy link

Jahans3 commented Jul 23, 2020

Describe the bug
Two related issues related to documentation: creating the relationships and querying them using DataStore.

The way the Amplify JS docs suggest creating one-to-many relationships seems a bit counterintuitive, it suggests using a combination of @connection and @key directives then storing IDs. Using this method they don't appear to have a genuine relationship - this is highlighted by the suggested way of querying relations, which simply involves querying all instances of a model with have the ID of their parent as a property and then filtering by parent ID on the client. Surely this isn't scalable. Why is this suggested over using the predicate argument of DataStore.query?

So I had a look at the sister Amplify libraries and the iOS one makes much more sense. Not only do they suggest a different way of defining relationships - two @connection directives (more intuitive, what I had initially thought was correct), but the way they show querying makes a lot more sense too.

The backend shouldn't care what front end library I use, so why are there two suggested methods? Which is correct?

Furthermore, with the querying, the Swift examples are exactly what I would expect:

Amplify.DataStore.query(Post.self, byId: "123") {
    switch $0 {
    case .success(let post):
        if let postWithComments = post {
            if let comments = postWithComments.comments {
                for comment in comments {
                    print(comment.content)
                }
            }
        } else {
            print("Post not found")
        }
    case .failure(let error):
        print("Post not found - \(error.localizedDescription)")
    }
}

But the JS version requires us to manually piece together any "related" models:

const post = await DataStore.query(Post, "123");
const comments = (await DataStore.query(Comment)).filter(c => c.postID === post.id);

What's going on here? Admittedly my knowledge of AppSync isn't amazing, it all comes from Amplify, which is perhaps why this is so confusing to me. But still I'd love the docs to be accurate since it's making developing using Amplify difficult at times.

And I'd love to know which is the correct method? 🙂

To Reproduce
See JS docs vs iOS docs

JS: https://docs.amplify.aws/lib/datastore/relational/q/platform/js

iOS: https://docs.amplify.aws/lib/datastore/relational/q/platform/ios

Thanks very much - sorry for not following the template, I felt a lot wasn't relevant to a documentation issue.

@swyxio
Copy link
Contributor

swyxio commented Jul 23, 2020

its a fair question and one i just ran into myself. appreciate the feedback!

@Jahans3
Copy link
Author

Jahans3 commented Jul 23, 2020

Thanks @sw-yx. Also sorry I probably wasn't as clear as I could have been - I was asking which method is correct?

Should I use two @connection directives or follow the @key + @connection method? If it's the former, then how can I query this using the JS SDK? It only ever returns the "one" portion of any one-to-many relationships. If it's the latter, then will I run into issues when building an iOS version of my app? Does each client side SDK require a different AppSync setup?

@Jahans3
Copy link
Author

Jahans3 commented Jul 23, 2020

So quick update, if I make the query in the AppSync console using the schema setup described in the iOS docs everything works exactly as expected. Which begs the question: since this is happening on the AppSync side, what is going on in the JS DataStore SDK to break this functionality?

Feels like a blocker, do I need to start removing DataStore from my app if I want this to work? @sw-yx

@swyxio
Copy link
Contributor

swyxio commented Jul 23, 2020

@Jahans3 straight answer is i dont know, i'm still pretty new to datastore myself. i'll refer this to the team and try to get an answer for you as im pretty interested in it myself. what @gsans told me was that both are different ways of tackling this same problem. the key + connection method uses dynamodb sort keys to group members together, whereas the connection + connection method nicely establishes the 2 way relationship and in theory could be infinitely recursive which matches my graphql mental model. i personally would much prefer the connection + connection method. not sure whether it doesnt work in DataStore, i need to repro to verify

@swyxio
Copy link
Contributor

swyxio commented Jul 23, 2020

quick answer from the team - yes, unfortunately in JS the key + connection method is recommended for now because of current limitations in lazyloading.

@Jahans3
Copy link
Author

Jahans3 commented Jul 23, 2020

That's a shame, but thanks for the quick update anyway @sw-yx

Is it OK if I query the "many" portions of my relations using the predicate argument (DataStore.query(Child, r => r.parentId('eq', parent.id))) instead of the way specified in the docs - is there a special reason they suggest filtering in the client?

@mauerbac
Copy link
Member

Thanks @Jahans3 and @sw-yx . Going to move this to our docs repo.

@mauerbac mauerbac transferred this issue from aws-amplify/amplify-js Jul 23, 2020
@swyxio
Copy link
Contributor

swyxio commented Jul 23, 2020

@Jahans3 unfortunately im gonna defer this qtn to the team because i dont know myself yet and dont want to give you the wrong answer. sorry. might also want to ask in the amplify discord too with more experts there

@Jahans3
Copy link
Author

Jahans3 commented Jul 23, 2020

@mauerbac might be worth creating a similar issue in the JS repo about this lazy loading/DataStore JS issue? Feel like this is a pretty major flaw

@sw-yx no problem, thanks for the suggestion. Cheers for the help today 👍

@swyxio
Copy link
Contributor

swyxio commented Jul 23, 2020

pretty sure there's an issue for the lazyloading one, i've seen it in a few places. im sure theres a v good reason why it hasnt been implemented yet

@mauerbac
Copy link
Member

Sorry for the delayed follow-up. We are tracking the lazy loading feature request here: aws-amplify/amplify-js#5054

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
amplify/js Issues tied to JS
Projects
None yet
Development

No branches or pull requests

4 participants