Skip to content
This repository has been archived by the owner on May 6, 2023. It is now read-only.

Rename resolve[d] to resolve[d]Mock #160

Open
saiichihashimoto opened this issue Sep 7, 2022 · 4 comments
Open

Rename resolve[d] to resolve[d]Mock #160

saiichihashimoto opened this issue Sep 7, 2022 · 4 comments

Comments

@saiichihashimoto
Copy link
Owner

saiichihashimoto commented Sep 7, 2022

As @mckelveygreg made clear in #158, some things about "resolve" are unclear:

  • s.resolved<...> should be the type but with references resolved. Right now it's both parsed AND resolved. These should be two separate types.
  • docType.resolve(mock) doesn't describe that it's for mocks, so it implies it does something you'd want in application code, which is untrue.
  • docType.resolve and docType.parse should be independent. Resolving also parses, which is unexpected. We should be able to do both to the same doc, but it's unclear how this API should go.

This was a consequence of the main use case I made this library for, which is not everyone's use case:

  • I query each document independently and fully. No expanded references, no projections, etc. Since I cache and traverse through application logic rather than in groq's syntax, none of this is an issue to me.
  • Parsing everything right after querying is easy and non-conflicing with any types.
  • Mocking for tests is easy, resolving the types there is mainly because my application logic does join these together and I'd like to have that in my tests.

However we go about fixing this, there's definitely breaking changes (which is fine). I think making s.resolved<...> the same as s.infer<...>, except references are the type of the resolved document should be easy.

The bigger issue is parsing. Parsing expects the value of the doc, and it essentially needs to handle a doc that has expanded references as well (and return the correct types). Expanded references are only one thing that a query can do that's "non-standard", which starts opening the door into parsing any valid query. Doing the work for this specific use case will likely cover a lot of use cases, but ultimately end up opening the door into individually solving the various groq query use cases.

Having a query builder that utilizes these types will get us type safe queries, typed outputs from those queries, and a parser that traverses all the way through to the query outputs. This will make the "typing docs with resolved references" an irrelevant problem: if we can type our queries, we'll type whichever references we happen to resolve, along with whatever else we want to do in our groq queries.

My big issue with this is groq is not small. Typing docs was relatively straightforward and not fragile, creating a typed query builder for everything groq can do is not going to easy or straightforward, will grow scope massively, and be prone to bugs.

@miklschmidt
Copy link
Contributor

miklschmidt commented Sep 7, 2022

I think it's okay to be "forced" to manually query the data in a way that complies with the schema's zod (if expanded references could be parsed as well). Query results are iffy to work with until they've been parsed and you can be sure of the shape. Ideally i'd like a way to parse a document with expanded references so you can query a full expanded document in one go. Maybe a docType.parseResolved?

Then you'd have:

  1. docType.parse parses the document (no expanded references)
  2. docType.resolveMock replaces references with their corresponding document mocks (no parsing).
  3. docType.parseResolved parses the document with expanded references.

@saiichihashimoto
Copy link
Owner Author

At this point, I think the normal inferred types and the parsed types are enough. I think the breaking change I need to make is renaming doc.resolve into doc.resolveMock so no one assumes that it does anything else. There's ultimately no doc.resolve in application code at all. The only use of it is to either translate inferred and/or parsed types into having expanded references. But would those expanded references be the inferred ones or the parsed ones? I'm imagining the inferred ones. But wanting to resolve even those isn't that unreasonable, but only sometimes. Even without addressing other groq use cases, resolving is hairy enough that I'm not going to address this in application code. If someone wants the types to do anything beyond being inferred from the schema or parsed from zod, they'll have to figure it out.

@saiichihashimoto saiichihashimoto changed the title Confusing resolved names Rename resolve[d] to resolve[d]Mock Jan 5, 2023
@miklschmidt
Copy link
Contributor

My workaround for this so far, and you're probably not gonna like this, is to use doc.resolve, zodResolved and zod.transform to look up references in a cached array containing all items of that type (fetched ahead of time). I'm using astro so it's all statically generated and thus doesn't hurt fetching everything upfront as i'm bound to fetch it anyway. So doc.resolve results in an entirely resolved document. It's not ideal for dynamically rendered things where you'd want to do all the fetching in a single groq query, but neither is not having a solution to this. It's a bit of a shame that groq makes it so easy to get the data, but parsing involves tons of juggling because the types don't match. Shooting a query everytime you need a reference that isn't resolved is really bad as well. Fact is, you need to handle references somehow, and there's currently no good way to do that, it needs a proper solution, imo.

@saiichihashimoto
Copy link
Owner Author

Yeah… I still believe the best solution for this should be to try and type groqs directly. The reality is that ‘ + = ’ is the only real truth. Resolution is just a common operation in groq, but by no means the only that should be handled. Parsing should only exist to be smart enough to know that, whenever we retrieve that specific field, we want to change its type, ie a datetime field should always produce a ‘Date’.

I think there’s a reality where ‘defineType’ gives enough hints to derive the basic types, where this library no longer needs to do that. If that’s true, it becomes a matter of giving some kind of ‘sanity-typed-groq-builder’ those types and let it drive the correct types from that.

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

No branches or pull requests

2 participants