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

Auto-population of mutation bodies #471

Closed
wants to merge 32 commits into from
Closed

Conversation

andyrichardson
Copy link
Contributor

@andyrichardson andyrichardson commented Nov 18, 2019

Fix #472

Todo

  • Create fragment definitions instead of inline fragments
  • Handle/consider issue where mutation is called but no "prior knowledge" is available
  • Allow populating fields with existing selection sets
  • Add support for interfaces
  • Only add fragments to the query when they are required (currently all user-provided fragments are appended)
  • Add support for recurqling (pronounced re-cur-cul)

Example

Kapture 2019-11-18 at 17 32 07

@andyrichardson andyrichardson added feature 🚀 future 🔮 An enhancement or feature proposal that will be addressed after the next release labels Nov 18, 2019
Copy link
Collaborator

@JoviDeCroock JoviDeCroock left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I like the notion this exchange introduces, I'm just a bit on the ropes if it belongs inside the repository, the thing that makes me reluctant is quite superficial since if unused this will be tree-shaken off but in most bundle-analyzers like bundlePhobia this will add a lot of size with the 2 imported methods.

@kitten kitten changed the title Auto-population of mutation arguments WIP: Auto-population of mutation arguments Nov 18, 2019
@imranolas imranolas changed the title WIP: Auto-population of mutation arguments WIP: Auto-population of mutation bodies Nov 18, 2019
@imranolas
Copy link
Contributor

#472 The rationale behind this for anyone that is interested

@andyrichardson andyrichardson changed the title WIP: Auto-population of mutation bodies Auto-population of mutation bodies Nov 18, 2019
@andyrichardson
Copy link
Contributor Author

andyrichardson commented Nov 19, 2019

I like the notion this exchange introduces, I'm just a bit on the ropes if it belongs inside the repository, the thing that makes me reluctant is quite superficial since if unused this will be tree-shaken off but in most bundle-analyzers like bundlePhobia this will add a lot of size with the 2 imported methods.

I hear you - but I don't think we should allow stats from services like bundlephobia to influence architectural decisions 😞

I'm not sure where else it would make sense to serve this exchange and I can see most Urql users wanting to use it! Did you have any alternatives in mind?

@kitten
Copy link
Member

kitten commented Nov 19, 2019

@andyrichardson From an architectural standpoint; this is super useful in combination with @urql/exchange-graphcache, so if that's separate, then having this separate makes sense, but there's some flexibility in that:

  • We can add it to @urql/exchange-graphcache. That's good since they'd be used together but bad because it's technically a separate thing and shouldn't be "hidden"
  • We can make @urql/exchange-graphcache into a monorepo that has some kind of name that implies that it makes urql into a full "fat" client. I do like that, because then we'd have a repo that basically says: "Hey, this is what you need if you're replacing Relay/Apollo"
  • We can make it a separate @urql/exchange-populate repo (don't mind the name)
  • We could move to a "fat" monorepo that includes the above and graphcache

@andyrichardson
Copy link
Contributor Author

I think grouping it with graphcache isn't a bad shout if the bundle size impact is too large.

While it can be used (and we should encourage use) with any cache, it doesn't make much sense to use this with the default cache.

@kitten
Copy link
Member

kitten commented Nov 19, 2019

Here's a discussion thread on Spectrum, so we can discuss this publicly but with a lot more messages. I suspect this is going to be a longer discussion 😆 https://spectrum.chat/urql/general/core-discussion-monorepo-more-repos~51619a7d-dedd-4b35-99ab-0085cf15e233

@andyrichardson andyrichardson added feature 🚀 and removed feature 🚀 future 🔮 An enhancement or feature proposal that will be addressed after the next release labels Nov 20, 2019
Copy link
Member

@kitten kitten left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Quick first pass; It looks great! There's a couple bits I don't quite get yet, and a couple that could use some comments :) overall just have a few questions and nits though, so all good

@andyrichardson
Copy link
Contributor Author

Merged to graphcache

@kitten kitten deleted the mutation-populate branch February 6, 2020 19:03
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

Successfully merging this pull request may close these issues.

RFC: Auto populate mutation body
4 participants