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

mutate works selectively unless I disable suspense #972

Closed
aaronfulkerson opened this issue Oct 8, 2020 · 24 comments · Fixed by blitz-js/blitz#1430
Closed

mutate works selectively unless I disable suspense #972

aaronfulkerson opened this issue Oct 8, 2020 · 24 comments · Fixed by blitz-js/blitz#1430

Comments

@aaronfulkerson
Copy link

aaronfulkerson commented Oct 8, 2020

What is the problem?

Mutate works selectively on arrays unless I disable suspense. Works fine on single objects.

Steps to Reproduce

  1. Call mutate with suspense enabled (default).

Versions

debug: blitzPkgPath: /Volumes/Samsung Portable SSD T3 Media/amweb-blitz/node_modules/blitz/dist/index.js
debug: cliPkgPath: /Volumes/Samsung Portable SSD T3 Media/amweb-blitz/node_modules/blitz/node_modules/@blitzjs/cli/lib/src/index.js 

macOS Catalina | darwin-x64 | Node: v14.13.0

blitz: 0.24.1 (global)
blitz: 0.24.1 (local)

  Package manager: npm 
  System:
    OS: macOS 10.15.7
    CPU: (4) x64 Intel(R) Core(TM) i3-8100B CPU @ 3.60GHz
    Memory: 226.90 MB / 16.00 GB
    Shell: 5.7.1 - /bin/zsh
  Binaries:
    Node: 14.13.0 - ~/.nvm/versions/node/v14.13.0/bin/node
    Yarn: Not Found
    npm: 6.14.8 - ~/.nvm/versions/node/v14.13.0/bin/npm
    Watchman: Not Found
  npmPackages:
    @prisma/cli: 2.8.1 => 2.8.1 
    @prisma/client: 2.8.1 => 2.8.1 
    blitz: 0.24.1 => 0.24.1 
    react: 0.0.0-experimental-7f28234f8 => 0.0.0-experimental-7f28234f8 
    react-dom: 0.0.0-experimental-7f28234f8 => 0.0.0-experimental-7f28234f8 
    typescript: 4.0.3 => 4.0.3

Other

Let's say I have an array of users with a company and I call console.log right after the line that contains useQuery. console.log should fire whenever the contents of the users array changes. It's curious to me that after calling mutate I'm seeing console.log fire but it still contains the contents of the old array. If I disable suspense I see console.log fire twice. The first time it contains the new user and the second time it's the old data plus the new user.

This behavior seems selective but I can't figure out when it works and when it doesn't. In my companies page, mutate works as expected but other pages require suspense to be disabled before mutate functions as expected.

This started after I upgraded to 0.24.1 and replaced my calls to mutations with useMutation.

@aaronfulkerson aaronfulkerson changed the title mutate not working on objects with relations mutate works selectively unless I disable suspense Oct 8, 2020
@aaronfulkerson
Copy link
Author

aaronfulkerson commented Oct 8, 2020

I think the reference to mutate might be changing but I'm not really sure. When I change the params of this query to undefined I don't have any problems but if the params change due to some other element of the page changing then mutate stops working.

// broken
const [users, { mutate }] = useQuery(getUsers, { companyId: company?.id, userQuery })

// works
const [users, { mutate }] = useQuery(getUsers, { companyId: undefined, userQuery: undefined })

Why does disabling suspense cause it to function as expected?

@flybayer
Copy link
Member

Wow this is an interesting one!!

I think definitely we need to change mutate to use useCallback internally. Maybe just doing that will fix this.

@aericson
Copy link
Contributor

aericson commented Oct 12, 2020

@aaronfulkerson I tried to reproduce this but couldn't. Are you sure you are not mutating the array, rather than passing a new one, by accident?

@aaronfulkerson
Copy link
Author

@aericson I'm not sure what you mean by mutating the array rather than passing a new one. To my knowledge the way you use mutate is by passing it a newly-created item so how I've been doing it when it comes to mutating arrays is passing the newly created item as a single-item array like so:

const item = await createItemMutation()
mutate([item])

And this method works when suspense is disabled. When suspense is enabled, however, it doesn't work if the query that the array comes from contains parameters that change.

@aericson
Copy link
Contributor

@aericson I'm not sure what you mean by mutating the array rather than passing a new one. To my knowledge the way you use mutate is by passing it a newly-created item so how I've been doing it when it comes to mutating arrays is passing the newly created item as a single-item array like so:

const item = await createItemMutation()
mutate([item])

And this method works when suspense is disabled. When suspense is enabled, however, it doesn't work if the query that the array comes from contains parameters that change.

Ah, I see. I thought this was something else. I thought you were doing optimistic update with the data returned by the useQuery.

@aaronfulkerson
Copy link
Author

aaronfulkerson commented Oct 17, 2020

I think I understand what's going on now. Whenever a parameter changes, react-query is creating a new cache, treating it as an entirely new query with new references to fetch and mutate. I'm not sure what the solution is and I don't understand why things "just work" when I disable suspense.

@flybayer
Copy link
Member

@aaronfulkerson can you fork this blitz codesandbox and make a reproduction of your problem? Currently I don't grasp exactly what the issue is.

(After changing the schema in the sandbox, click "Restart Sandbox" under the "Server Control Panel" button on the far left sidebar.)

@aaronfulkerson
Copy link
Author

aaronfulkerson commented Oct 19, 2020

@flybayer https://codesandbox.io/s/friendly-cache-4tkmz

Not everything is there but if you go to the companies page and add a new company, everything works as expected. Now go to the projects page, select a company and create a new project--mutate doesn't work. Disable suspense on the getProjects query--mutate works. Seems like the difference is one query has params that change and one doesn't. I think the references to mutate/refetch are changing.

@goldo
Copy link

goldo commented Oct 19, 2020

Hi! I'm having the same issue and it seems to be resolved when suspense is disabled.

Context of my problem:

I have a list of user and a list of games, and I linked games to users.
getUserGames retrieve the user’s games
the user can add Games to his list, the mutation seems fine. But when I mutate the initial query, the query is launched, but its not used, react query seem to return the cache without using the network request that just finished. It seems weird

I tried to add an await in front of the mutate() but still no success. It seems to work after.. a hot reload refresh. Does this mean this could be a problem with SSR ? (edited)

I also tried to use refetch but same problem.

  const currentUser = useCurrentUser()
  const [userGamesResult, { mutate }] = useQuery(getUserGames, {
    where: { userId: currentUser?.id },
  })  
  const [createUserGameMutation] = useMutation(createUserGame)const { userGames } = userGamesResultconst addUserGame = ({ link }) => async ({ gameId }) => {
    try {
      await createUserGameMutation({
        data: {
          user: { connect: { id: currentUser?.id } },
          game: { connect: { id: gameId } },
          link,
        },
      })
      mutate(userGamesResult)
    } catch (error) {
      console.log(error)
      alert(error.message)
    }
  }

@flybayer
Copy link
Member

Update: I haven't figured out the root problem, but I have figured out a weird workaround that makes everything work:

-const [projects, { setQueryData }] = useQuery(getProjects,{ companyId: company?.id })
+const [projects, { setQueryData }] = useQuery(getProjects,{ companyId: company?.id }, {})

Without that extra empty config object, the page never re-renders and useQuery never gets called, which is why you never see the new data. The new data exists in the cache properly, it's just not being rendered.

It's also interesting that this bug doesn't exist when running the local dev version of blitz.

Btw, @aaronfulkerson, your usage of mutate (now setQueryData) has bug. instead of setting a single array, pass a function that gets the existing array and adds to it (in case of create project)

-setQueryData([project])
+setQueryData((projects) => [...projects, project])

@flybayer
Copy link
Member

AAAAH! Looks like this was a bug in experimental react! Upgrading react and react-dom to 0.0.0-experimental-4ead6b530 appears to fix this!

Please try that and report back.

@aaronfulkerson
Copy link
Author

aaronfulkerson commented Oct 22, 2020

Update: I haven't figured out the root problem, but I have figured out a weird workaround that makes everything work:

-const [projects, { setQueryData }] = useQuery(getProjects,{ companyId: company?.id })
+const [projects, { setQueryData }] = useQuery(getProjects,{ companyId: company?.id }, {})

Without that extra empty config object, the page never re-renders and useQuery never gets called, which is why you never see the new data. The new data exists in the cache properly, it's just not being rendered.

It's also interesting that this bug doesn't exist when running the local dev version of blitz.

Btw, @aaronfulkerson, your usage of mutate (now setQueryData) has bug. instead of setting a single array, pass a function that gets the existing array and adds to it (in case of create project)

-setQueryData([project])
+setQueryData((projects) => [...projects, project])

Yeah I figured that out a couple of hours ago. When you use mutate on an array you need the old data too. The reason it "worked" previously is because it was refetching the data.

@aaronfulkerson
Copy link
Author

aaronfulkerson commented Oct 22, 2020

AAAAH! Looks like this was a bug in experimental react! Upgrading react and react-dom to 0.0.0-experimental-4ead6b530 appears to fix this!

Please try that and report back.

No luck here. Neither mutate nor refetch work after updating React. Disabling suspense still works, empty object does not.

@aaronfulkerson
Copy link
Author

Even after updating to 0.25.0-canary.0 I'm still not able to get refetch working. I switched to refetch after realizing mutate didn't work the way I thought it did although passing a function to setQueryData looks really good.

@goldo
Copy link

goldo commented Oct 22, 2020

same here, mutate() and refetch() not working with suspense on this. With the latest canary of blitz, I couldn't make setQueryData work quickly and refetch was not working either

@flybayer
Copy link
Member

Finally some I made some progress! I'm so sorry for the delay on this. Was traveling last week and some other life stuff to deal with.

Root Cause

react-query 2.7.0 broke this.

Here is the open react-query issue for this bug: TanStack/query#1160

Workaround

Add this to package.json and then run yarn again

  "resolutions": {
    "react-query": "2.6.0"
  },

Btw, I realized that adding {} worked for me simply because of HMR which revived the query subscription.

@aaronfulkerson
Copy link
Author

@flybayer should that work? It doesn't. Is the version in my project's package.json getting overridden by the version Blitz is using?

@flybayer
Copy link
Member

@aaronfulkerson Yes, that works as long as you are using yarn and not npm. That resolutions config will override the version that comes with blitz.

Here's the sandbox showing it working: https://codesandbox.io/s/adoring-kowalevski-gf0vc?file=/package.json

@aaronfulkerson
Copy link
Author

Didn't realize I had to use yarn. Thank you for taking care of this.

@flybayer
Copy link
Member

Yeah, npm should definitely support that feature too

@aaronfulkerson
Copy link
Author

Still doesn't work, I give up. Maybe @goldo can give it a shot.

@flybayer
Copy link
Member

flybayer commented Oct 28, 2020

@aaronfulkerson can you run yarn why react-query and make sure there's only one react-query version installed and that it's 2.6.0?

I can pair with you for a bit to help get it working if you want.

@goldo
Copy link

goldo commented Oct 28, 2020

@flybayer it does solve the problem!! 🙌 thanks a lot!

@flybayer
Copy link
Member

I'm planning to revert the bundled version back to 2.6.0 for now

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging a pull request may close this issue.

5 participants