Skip to content
This repository was archived by the owner on Jan 10, 2025. It is now read-only.

[graphql-config-utilities] Support multiple schema definitions (+ globs) #2367

Open
2 tasks done
notjosh opened this issue Aug 8, 2022 · 2 comments
Open
2 tasks done

Comments

@notjosh
Copy link
Contributor

notjosh commented Aug 8, 2022

Overview

graphql-config updated their spec for the schema field (since v3? v4?) to allow multiple files and globs.

Our current model within graphql-config-utilities assumes a single schema file.

Motivation

On Shop app, we're using graphql-typescript-definitions to generate the types from schema files. We use multiple schema files, so currently we have to run a script to merge our schema files. I was investigating skipping this skip and keeping multiple schemas when I ran into this problem.

I currently have a pretty simple patch as a workaround for multiple files, but haven't extended it to globs. It works, but it's a pretty large patch, so I'd rather it were fixed upstream.

One alternative might be ditching graphql-config-utilities from within graphql-typescript-definitions, and just using graphql-config directly to do the config parsing. Superficially they have a fairly comparable API surface, so this shouldn't be too hard.

Similarly, we could just keep graphql-config-utilities as a thin wrapper around graphql-config, adding a couple of functions as enhancements if we're using it frequently across packages. This shouldn't impact existing packages, and reduces our code to maintain.

(Or maybe both? We can put graphql-config-utilities into a deprecated state, but update it for any current consumers)

I can carve out some time for a PR if you have an opinions on the best direction to go here.

Thoughts?


Checklist

  • Please delete the labels section before submitting your issue
  • I have described this issue in a way that is actionable (if possible)
@BPScott
Copy link
Member

BPScott commented Aug 9, 2022

Back in the day graphql-config@v2 wasn't very strict with the data shapes that it returned - it didn't do much normalisation between single-project / multi project configs or handle the concept of 'default' projects. graphql-config-utilities was borne out of that fluffyness, in an attempt to standardise what graphql-config returned. These days graphql-config@v4 seems to a lot better at structuring what it returns and behaving consistently between single project and multi-project setups. We've updated to use v4 but haven't investigated what parts of graphql-config-utilities are no longer needed as part of that update. Theoretically the need for graphql-config-utilities has shrunk, and instead we should be able to leverage graphql-config directly in more places in graphql-typescript-definitions (and graphql-validate-fixtures) without needing to use graphql-config-utilities. However we've not had the time to prioritize that work as it pretty low impact.

One alternative might be ditching graphql-config-utilities from within graphql-typescript-definitions, and just using graphql-config directly to do the config parsing. Superficially they have a fairly comparable API surface, so this shouldn't be too hard.

I like the idea. I'd be interested to see a PR that makes graphql-typescript-definitions no longer depend upon graphql-config-utilities if that is easily doable. If that becomes pretty complicated then keeping graphql-config-utilities sounds worthwhile

@notjosh
Copy link
Contributor Author

notjosh commented Aug 15, 2022

@BPScott I've put up a draft at #2374, which doesn't look too bad (most of the diff is noise from fixtures).

In its current state, the PR handles both modern and legacy configs, so it maintains a dependency on graphql-config-utilities. I've got a couple of questions in the PR about how we could handle that, if we want to remove it entirely.

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

Successfully merging a pull request may close this issue.

2 participants