aws-amplify / amplify-js

A declarative JavaScript library for application development using cloud services.
https://docs.amplify.aws/lib/q/platform/js
Apache License 2.0
9.4k stars 2.11k forks source link

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

Open m9rc1n opened 2 months ago

m9rc1n commented 2 months ago

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

chrisbonifacio commented 2 months ago

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.

m9rc1n commented 2 months ago

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 commented 2 months ago

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 commented 2 months ago

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

amuresia commented 1 month ago

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.