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

ObserveQuery / OnUpdate not respecting selection set nested models - returns function instead #13267

Closed
3 tasks done
m9rc1n opened this issue Apr 19, 2024 · 11 comments
Closed
3 tasks done
Assignees
Labels
bug Something isn't working Gen 2 Issues related to Gen 2 Amplify projects GraphQL Related to GraphQL API issues

Comments

@m9rc1n
Copy link

m9rc1n commented Apr 19, 2024

Before opening, please confirm:

JavaScript Framework

React

Amplify APIs

GraphQL API, DataStore

Amplify Version

v6

Amplify Categories

api

Backend

Amplify Gen 2 (Preview)

Environment information

  System:
    OS: macOS 14.4.1
    CPU: (10) arm64 Apple M1 Pro
    Memory: 1.50 GB / 16.00 GB
    Shell: 5.9 - /bin/zsh
  Binaries:
    Node: 20.11.0 - ~/.nvm/versions/node/v20.11.0/bin/node
    Yarn: 1.22.18 - /opt/homebrew/bin/yarn
    npm: 10.3.0 - ~/.nvm/versions/node/v20.11.0/bin/npm
  Browsers:
    Chrome: 123.0.6312.124
    Safari: 17.4.1
  npmPackages:
    %name%:  0.1.0 
    @ampproject/toolbox-optimizer:  undefined ()
    @aws-amplify/backend: ^0.13.0-beta.20 => 0.13.0-beta.20 
    @aws-amplify/backend-cli: ^0.12.0-beta.22 => 0.12.0-beta.22 
    @aws-amplify/ui-react: ^6.1.8 => 6.1.8 
    @aws-amplify/ui-react-internal:  undefined ()
    @babel/core:  undefined ()
    @babel/runtime:  7.22.5 
    @edge-runtime/cookies:  4.1.0 
    @edge-runtime/ponyfill:  2.4.2 
    @edge-runtime/primitives:  4.1.0 
    @hapi/accept:  undefined ()
    @mswjs/interceptors:  undefined ()
    @napi-rs/triples:  undefined ()
    @next/font:  undefined ()
    @next/react-dev-overlay:  undefined ()
    @opentelemetry/api:  undefined ()
    @types/node: ^20 => 20.12.7 
    @types/react: ^18 => 18.2.79 
    @types/react-dom: ^18 => 18.2.25 
    @vercel/nft:  undefined ()
    @vercel/og:  0.6.2 
    acorn:  undefined ()
    amphtml-validator:  undefined ()
    anser:  undefined ()
    arg:  undefined ()
    assert:  undefined ()
    async-retry:  undefined ()
    async-sema:  undefined ()
    aws-amplify: ^6.0.28 => 6.0.28 
    aws-amplify/adapter-core:  undefined ()
    aws-amplify/analytics:  undefined ()
    aws-amplify/analytics/kinesis:  undefined ()
    aws-amplify/analytics/kinesis-firehose:  undefined ()
    aws-amplify/analytics/personalize:  undefined ()
    aws-amplify/analytics/pinpoint:  undefined ()
    aws-amplify/api:  undefined ()
    aws-amplify/api/server:  undefined ()
    aws-amplify/auth:  undefined ()
    aws-amplify/auth/cognito:  undefined ()
    aws-amplify/auth/cognito/server:  undefined ()
    aws-amplify/auth/enable-oauth-listener:  undefined ()
    aws-amplify/auth/server:  undefined ()
    aws-amplify/data:  undefined ()
    aws-amplify/data/server:  undefined ()
    aws-amplify/datastore:  undefined ()
    aws-amplify/in-app-messaging:  undefined ()
    aws-amplify/in-app-messaging/pinpoint:  undefined ()
    aws-amplify/push-notifications:  undefined ()
    aws-amplify/push-notifications/pinpoint:  undefined ()
    aws-amplify/storage:  undefined ()
    aws-amplify/storage/s3:  undefined ()
    aws-amplify/storage/s3/server:  undefined ()
    aws-amplify/storage/server:  undefined ()
    aws-amplify/utils:  undefined ()
    aws-cdk: ^2.138.0 => 2.138.0 
    aws-cdk-lib: ^2.138.0 => 2.138.0 
    babel-packages:  undefined ()
    browserify-zlib:  undefined ()
    browserslist:  undefined ()
    buffer:  undefined ()
    bytes:  undefined ()
    ci-info:  undefined ()
    cli-select:  undefined ()
    client-only:  0.0.1 
    comment-json:  undefined ()
    compression:  undefined ()
    conf:  undefined ()
    constants-browserify:  undefined ()
    constructs: ^10.3.0 => 10.3.0 
    content-disposition:  undefined ()
    content-type:  undefined ()
    cookie:  undefined ()
    cross-spawn:  undefined ()
    crypto-browserify:  undefined ()
    css.escape:  undefined ()
    data-uri-to-buffer:  undefined ()
    debug:  undefined ()
    devalue:  undefined ()
    domain-browser:  undefined ()
    edge-runtime:  undefined ()
    esbuild: ^0.20.2 => 0.20.2 (0.19.12)
    eslint: ^8 => 8.57.0 
    eslint-config-next: 14.1.4 => 14.1.4 
    events:  undefined ()
    find-cache-dir:  undefined ()
    find-up:  undefined ()
    fresh:  undefined ()
    get-orientation:  undefined ()
    glob:  undefined ()
    gzip-size:  undefined ()
    http-proxy:  undefined ()
    http-proxy-agent:  undefined ()
    https-browserify:  undefined ()
    https-proxy-agent:  undefined ()
    icss-utils:  undefined ()
    ignore-loader:  undefined ()
    image-size:  undefined ()
    is-animated:  undefined ()
    is-docker:  undefined ()
    is-wsl:  undefined ()
    jest-worker:  undefined ()
    json5:  undefined ()
    jsonwebtoken:  undefined ()
    loader-runner:  undefined ()
    loader-utils:  undefined ()
    lodash.curry:  undefined ()
    lru-cache:  undefined ()
    micromatch:  undefined ()
    mini-css-extract-plugin:  undefined ()
    nanoid:  undefined ()
    native-url:  undefined ()
    neo-async:  undefined ()
    next: 14.1.4 => 14.1.4 
    node-fetch:  undefined ()
    node-html-parser:  undefined ()
    ora:  undefined ()
    os-browserify:  undefined ()
    p-limit:  undefined ()
    path-browserify:  undefined ()
    platform:  undefined ()
    postcss-flexbugs-fixes:  undefined ()
    postcss-modules-extract-imports:  undefined ()
    postcss-modules-local-by-default:  undefined ()
    postcss-modules-scope:  undefined ()
    postcss-modules-values:  undefined ()
    postcss-preset-env:  undefined ()
    postcss-safe-parser:  undefined ()
    postcss-scss:  undefined ()
    postcss-value-parser:  undefined ()
    process:  undefined ()
    punycode:  undefined ()
    querystring-es3:  undefined ()
    raw-body:  undefined ()
    react: ^18 => 18.2.0 
    react-builtin:  undefined ()
    react-dom: ^18 => 18.2.0 
    react-dom-builtin:  undefined ()
    react-dom-experimental-builtin:  undefined ()
    react-experimental-builtin:  undefined ()
    react-is:  18.2.0 
    react-refresh:  0.12.0 
    react-server-dom-turbopack-builtin:  undefined ()
    react-server-dom-turbopack-experimental-builtin:  undefined ()
    react-server-dom-webpack-builtin:  undefined ()
    react-server-dom-webpack-experimental-builtin:  undefined ()
    regenerator-runtime:  0.13.4 
    sass-loader:  undefined ()
    scheduler-builtin:  undefined ()
    scheduler-experimental-builtin:  undefined ()
    schema-utils:  undefined ()
    semver:  undefined ()
    send:  undefined ()
    server-only:  0.0.1 
    setimmediate:  undefined ()
    shell-quote:  undefined ()
    source-map:  undefined ()
    stacktrace-parser:  undefined ()
    stream-browserify:  undefined ()
    stream-http:  undefined ()
    string-hash:  undefined ()
    string_decoder:  undefined ()
    strip-ansi:  undefined ()
    superstruct:  undefined ()
    tar:  undefined ()
    terser:  undefined ()
    text-table:  undefined ()
    timers-browserify:  undefined ()
    tsx: ^4.7.2 => 4.7.2 
    tty-browserify:  undefined ()
    typescript: ^5.4.5 => 5.4.5 (4.4.4, 4.9.5)
    ua-parser-js:  undefined ()
    unistore:  undefined ()
    util:  undefined ()
    vm-browserify:  undefined ()
    watchpack:  undefined ()
    web-vitals:  undefined ()
    webpack:  undefined ()
    webpack-sources:  undefined ()
    ws:  undefined ()
    zod:  undefined ()
  npmGlobalPackages:
    @aws-amplify/cli: 12.10.3
    aws-sso-creds-helper: 1.12.0
    corepack: 0.23.0
    npm: 10.3.0


Describe the bug

Nested model is replaced with function after it is delivered by observeQuery or onUpdate.
Selection Set setting is not respected by observeQuery or onUpdate.

Expected behavior

Selection Set setting will work the same for observeQuery as it does for other functions i.e. get, or list.

Reproduction steps

  1. Follow https://docs.amplify.aws/gen2/start/quickstart/nextjs-app-router-client-components/
  2. Update data/resource.ts
  3. Update components/TodoList.tsx
  4. Open browser
  5. Create todo
  6. Refresh page
  7. Create another todo - you should see list of todos - one will contain content object, another content object as function.

Code Snippet

const schema = a.schema({
  Todo: a
    .model({
      title: a.string(),
      content: a.hasOne("Content", "todoId"),
    })
    .authorization([a.allow.public()]),
  Content: a
    .model({
      todoId: a.id(),
      todo: a.belongsTo("Todo", "todoId"),
      text: a.string(),
    })
    .authorization([a.allow.public()]),
})
// components/TodoList.tsx
"use client"

import type { Schema } from "@/amplify/data/resource"
import type { SelectionSet } from "aws-amplify/data"
import { generateClient } from "aws-amplify/data"
import { useEffect, useState } from "react"

// generate your data client using the Schema from your backend
const client = generateClient<Schema>()

const selectionSet = ["id", "title", "content.*"] as const

type TodoModel = SelectionSet<Schema["Todo"], typeof selectionSet>

export default function TodoList() {
  const [todos, setTodos] = useState<TodoModel[]>([])

  async function listTodos() {
    // fetch all todos
    const { data } = await client.models.Todo.list({
      selectionSet: selectionSet,
    })
    console.log("After Page Load", data)
    setTodos(data)
  }

  useEffect(() => {
    listTodos()
  }, [])

  useEffect(() => {
    const sub = client.models.Todo.observeQuery({
      selectionSet: selectionSet,
    }).subscribe(({ items }) => {
      console.log("After Adding New Item", items)
      setTodos([...items])
    })

    return () => sub.unsubscribe()
  }, [])

  return (
    <div>
      <h1>Todos</h1>
      <button
        onClick={async () => {
          const { data: newTodo } = await client.models.Todo.create({
            title: "Title",
          })
          await client.models.Content.create({
            text: "Content",
            todoId: newTodo.id,
          })
        }}
      >
        Create
      </button>

      <ul>
        {todos.map((todo) => (
          <li key={todo.id}>
            {todo.title} - {todo.content.text}
          </li>
        ))}
      </ul>
    </div>
  )
}

Log output

Screenshot 2024-04-18 at 22 39 11

aws-exports.js

No response

Manual configuration

No response

Additional configuration

No response

Mobile Device

No response

Mobile Operating System

No response

Mobile Browser

No response

Mobile Browser Version

No response

Additional information and screenshots

No response

@m9rc1n m9rc1n added the pending-triage Issue is pending triage label Apr 19, 2024
@cwomack cwomack added the GraphQL Related to GraphQL API issues label Apr 19, 2024
@chrisbonifacio
Copy link
Member

chrisbonifacio commented Apr 23, 2024

Hi @m9rc1n it looks like you are using observeQuery on the Todo model. Although you are creating a Todo and Content record subsequently, the initial creation of a Todo record will trigger the subscription but the data it receives does not yet contain the relationship. The creation of the Content record will not trigger the subscription either.

So, the function in place of the new record's content field is actually a promise that can be awaited to lazy load the related Content data.

@chrisbonifacio chrisbonifacio added question General question and removed pending-triage Issue is pending triage labels Apr 23, 2024
@m9rc1n
Copy link
Author

m9rc1n commented Apr 24, 2024

Hi @chrisbonifacio,

Thanks for looking into this issue.

I see your point. On the other hand, this design decision might cause confusion among end users (i.e. how to use nested fields in subscriptions?) and lead to some race conditions.

For instance, if I do:

  useEffect(() => {
    const sub = client.models.Todo.observeQuery({
      selectionSet: selectionSet,
    }).subscribe(async ({ items }) => {
      console.log("After Adding New Item", items)
      for (const item of items) {
        if (typeof item.content == "function") {
          const { data: content} = await item.content()
          console.log("Updated content" , content)
        }
      }
      setTodos([...items])
    })

    return () => sub.unsubscribe()
  }, [])

Then the content will be still undefined as subscription Todo arrives faster than Content update is completed.

Also I was able to recreate this issue using reverse subscription - listening to updates on Content instead of Todo. In this case Todo is created first (awaited) and todoId is being passed as argument to Content, so that race condition shouldn't be a problem.

@chrisbonifacio chrisbonifacio added bug Something isn't working Gen 2 Issues related to Gen 2 Amplify projects labels Apr 29, 2024
@chrisbonifacio
Copy link
Member

chrisbonifacio commented Apr 29, 2024

Hi @m9rc1n we're going to classify this as a bug at the moment, as this probably needs some discussion. Returning the relationship before it exists won't be possible via the subscription so we will look into improving this experience, at least avoiding confusion around related data in messages from onCreate subscriptions.

@m9rc1n
Copy link
Author

m9rc1n commented Apr 30, 2024

Hi @chrisbonifacio, sure thing.
Let me know if you need some testing or help investigating / discussing over this issue.

@chrisbonifacio chrisbonifacio removed the question General question label May 13, 2024
@amuresia
Copy link

amuresia commented Jun 7, 2024

It is worth noting that typescript errors pop up if an onCreate or onUpdate subscription uses a selection set and we try to lazy load the relations because they're not typed as functions.

@svidgen
Copy link
Member

svidgen commented Jul 24, 2024

Hi @m9rc1n, it looks like you're using the beta versions of backend (and the version of aws-amplify that contains the beta version of this functionality). Can you get all of your amplify-related libraries up to date and try again?

Edited: I originally thought I demo'd working behavior on my end; but missed part of the issue/repro steps. 😓

@svidgen svidgen added question General question pending-response bug Something isn't working and removed bug Something isn't working question General question labels Jul 24, 2024
@svidgen
Copy link
Member

svidgen commented Jul 24, 2024

Sorry for the jumping the gun on a response above. My brain definitely dropped some context between when I popped this issue open and when I returned to really look at it. 😓 So, I want to clarify one thing in particular: Updating to the latest versions should 100% be done, but will not fix this.

Some of the complexity here is that graphql subscriptions can only receive fields present on the corresponding mutations. In addition to the chicken-egg problem @chrisbonifacio noted, which would probably be best addressed by simply "touching" relevant models to broadcast a "refresh", the more complicated side of this is aligning selection sets between mutations and subscriptions.

And that's really the stickier point without a clear solution here. If you subscribe to fields {a b c { x y z }} but the mutation's selection set doesn't contain c { x y z}, the subscription cannot in principle ever see those fields. So, that's what we're mulling over internally.

@Rat-fi
Copy link

Rat-fi commented Aug 10, 2024

i found an alternative that works well for my issue, maybe will work for you try something like this:
my ReservationCard and paiements would be respectively your Todo and your Content
my use case is a bit different since i only want the first value but you could do it for all your Todos

useEffect(() => {
        const sub = client.models.ReservationCard.observeQuery({
            filter: {
                playerId: {
                    eq: contact.id,
                },
            },
        }).subscribe({
            next: async ({ items, isSynced }) => {
                const currentCard = items[0];
                if (typeof currentCard.paiements === 'function') {
                    const { data: resolvedPaiements } = await currentCard.paiements();
                    setCard({ ...currentCard, paiements: resolvedPaiements });
                } else if (Array.isArray(currentCard.paiements)) {
                    setCard(currentCard);
                } else {
                    setCard({ ...currentCard, paiements: [] });
                }
            },
        });
    
        return () => sub.unsubscribe();
    }, [contact.id]);

@deepanswer
Copy link

i found an alternative that works well for my issue, maybe will work for you try something like this: my ReservationCard and paiements would be respectively your Todo and your Content my use case is a bit different since i only want the first value but you could do it for all your Todos

useEffect(() => {
        const sub = client.models.ReservationCard.observeQuery({
            filter: {
                playerId: {
                    eq: contact.id,
                },
            },
        }).subscribe({
            next: async ({ items, isSynced }) => {
                const currentCard = items[0];
                if (typeof currentCard.paiements === 'function') {
                    const { data: resolvedPaiements } = await currentCard.paiements();
                    setCard({ ...currentCard, paiements: resolvedPaiements });
                } else if (Array.isArray(currentCard.paiements)) {
                    setCard(currentCard);
                } else {
                    setCard({ ...currentCard, paiements: [] });
                }
            },
        });
    
        return () => sub.unsubscribe();
    }, [contact.id]);

Hi Rat-fi, what type are you using for currentCard?

@Rat-fi
Copy link

Rat-fi commented Aug 18, 2024

i found an alternative that works well for my issue, maybe will work for you try something like this: my ReservationCard and paiements would be respectively your Todo and your Content my use case is a bit different since i only want the first value but you could do it for all your Todos

useEffect(() => {
        const sub = client.models.ReservationCard.observeQuery({
            filter: {
                playerId: {
                    eq: contact.id,
                },
            },
        }).subscribe({
            next: async ({ items, isSynced }) => {
                const currentCard = items[0];
                if (typeof currentCard.paiements === 'function') {
                    const { data: resolvedPaiements } = await currentCard.paiements();
                    setCard({ ...currentCard, paiements: resolvedPaiements });
                } else if (Array.isArray(currentCard.paiements)) {
                    setCard(currentCard);
                } else {
                    setCard({ ...currentCard, paiements: [] });
                }
            },
        });
    
        return () => sub.unsubscribe();
    }, [contact.id]);

Hi Rat-fi, what type are you using for currentCard?

ReservationCard: a.model({
        playerId: a.id().required(),
        owner: a.belongsTo('Player', 'playerId'),
        paiements: a.hasMany('RefillCard', 'reservationCardId'),
        leftHours: a.integer().required(),
        code: a.string().required()

    }).authorization(allow => [
        allow.group('Admins')
    ]),
    ```
    
    and 
    type tempReservationCard = { id: string, playerId: string, leftHours: number, code: string, paiements?: RefillCard[] | (() => Promise<{ data: RefillCard[] }>) }
    
    const [card, setCard] = useState<tempReservationCard>({ id: '', playerId: '', code: '', leftHours: 0, paiements: [] });

@svidgen
Copy link
Member

svidgen commented Sep 11, 2024

Hey folks, a few fixes and docs sites updates have been submitted now that I believe address the problems identified in this thread. Here's a summary:

✅ Fixed: Custom selection sets on mutations

The selectionSet parameter on mutations had runtime support, but the types didn't indicate it was available. This has been fixed.

✅ Fixed: Custom selection sets containing related models on subscriptions

The types were correct for custom selection sets on subscriptions (including observeQuery), but the onCreate, onUpdate, and onDelete handlers weren't handling results correctly and were overwriting eagerly loaded related models with their respective lazy loaders. This has been fixed.

✅ Docs Updated: Subscription selection set limitations and patterns

Even with the aforementioned fixes, some of the code samples on this issue will not work as desired. Most notably, a subscription can only select fields provided to it via the corresponding mutation's selection set, and specifically for mutations that directly touch that specific model. In other words, if you want to subscribe to Todo updates with ["id", "title", "content.*"], the corresponding mutation must mutate the Todo and select ["id", "title", "content.*"] at a minimum.

Documentation has been updated to explain this with examples. Those updates can be found under the "Set up real-time list query" heading in the "Missing real-time events and model fields" Troubleshooting accordion/dropdown.

Next Steps: Update your apps

  1. Update to @aws-amplify/data-schema@^1.5.1 by running npm update @aws-amplify/data-schema. (It wouldn't hurt to update all amplify deps while you're at it!)
  2. Ensure all mutation selection sets are strict supersets of all corresponding subscription selection sets.
  3. Add "touch" mutations to every observed/subscribed-to record just after a related model is mutated in every case where you want the related model change to be reflected in the subscription.

Hope that helps!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working Gen 2 Issues related to Gen 2 Amplify projects GraphQL Related to GraphQL API issues
Projects
None yet
Development

No branches or pull requests

7 participants