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

Support code-splitting mutation operations with useMutation #4827

Open
alex-statsig opened this issue Oct 18, 2024 · 2 comments
Open

Support code-splitting mutation operations with useMutation #4827

alex-statsig opened this issue Oct 18, 2024 · 2 comments

Comments

@alex-statsig
Copy link
Contributor

Almost all mutations are not needed for the initial render of a page, often being triggered by some delayed user interaction like a button click. However, it is currently hard to avoid bloating bundles with the mutation nodes when using the built-in useMutation hook. The mutation nodes contain JS proportional to the mutation document size (and thus potentially proportional to the page size if the page's data is included as a fragment) in the query text and operation/fragment fields.

There's two main problems I've run into trying to solve this:

  1. useMutation requires a ConcreteRequest (GraphQLTaggedNode) as an argument. Since hooks cannot be called conditionally, this means the operation must be imported before the component using the hook can render. This makes it tricky to code-split the mutation's node from the component which triggers it. A possible fix here is to accept a Promise/JSResource for the node in the hook. When the commit function is called, it would chain off the promise/resource (potentially delaying the mutation request slightly if the resource hasn't resolved). This resource could be optimistically prefetched, or deferred until the user interacts (likely up to the caller to decide based on the tradeoff of mutation likelihood and desired speed of issuing the mutation)
  2. While it is possible to use persisted queries to avoid including the entire operation text in the bundle, the ConcreteRequest's fragment/operation fields still contains the AST of the mutation. This is inevitable to parse the response of the mutation, but not to kick off the mutation. The @preloadable annotation does allow generating a separate file containing just the parameters needed to issue the request, which can already be used to decrease bundle sizes with useQueryLoader+usePreloadedQuery. A possible fix here is to extend support to mutations, and add a "useDeferredMutation" hook which accepts Mutation$Parameters (PreloadableConcreteRequest), as well as a promise/JSResource for the actual mutation node (similar to useQueryLoader, or perhaps useEntryPointLoader)

While it would be possible to roll my own version of useMutation which allows this (the logic doesn't seem too complicated), I think it would be beneficial to provide first-class support to encourage this pattern. I'd be happy to help contribute something for this, but would like some confirmation that these are the right paths to go down

@captbaritone
Copy link
Contributor

Yes! This is a much needed ability, I'm fully in favor of finding a way for us to support this. In fact, it may be that this should be the recommended default way to use mutations. Given that, we should think carefully about what would be the best API here to make it easy to get right and low-friction.

One significant challenge is that today the babel transform blindly converts the graphql tagged template literal into a require of the generated artifact which is the bit we want to avoid eagerly loading. And yet, defining the mutation directly in the hook that's going to dispatch it is the best low-friction developer experience.

I could also see a few options around when we load the artifact:

  1. Fully lazily (when the mutation is dispatched by the user)
  2. Via hints from the user (we could expose a function from the hook to fetch the artifact and the user could call that, for example, when the user mouses over the button that would trigger the mutation)
  3. As soon as possible. The artifact could be omitted from the main bundle, but fetched as soon as the main bundle executes.

There are a lot of options here, so probably the next step would be to brainstorm. What timezone are you in? Would you be interested in attending one of our design discussion meetings to discuss what options we have here and decide what the next concrete steps would be?

I think this is really important work and would love to help support you get this feature added to Relay!

@alex-statsig
Copy link
Contributor Author

One significant challenge is that today the babel transform blindly converts the graphql tagged template literal into a require of the generated artifact which is the bit we want to avoid eagerly loading. And yet, defining the mutation directly in the hook that's going to dispatch it is the best low-friction developer experience.

Ah yeah I hadn't factored this in. Needing to define that in a separate file is definitely tedious (on top of likely defining a hook per mutation in its own file to capture common response handling logic)

There are a lot of options here, so probably the next step would be to brainstorm. What timezone are you in? Would you be interested in attending one of our design discussion meetings to discuss what options we have here and decide what the next concrete steps would be?

I am in PT, I'd be happy to join a design discussion meeting on this!

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

No branches or pull requests

2 participants