-
Notifications
You must be signed in to change notification settings - Fork 53
feat: Support globs and arrays of paths/globs for the schema
option and generate-schema
command arguments
#287
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
base: main
Are you sure you want to change the base?
Conversation
🦋 Changeset detectedLatest commit: fd4e02f The changes in this PR will be included in the next version bump. Not sure what this means? Click here to learn what changesets are. Click here if you're a maintainer who wants to add another changeset to this PR |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Hi,
Will there also be support for an LSP plugin?
In my current use case, I’m relying on the introspection endpoint. That works, but it requires the server to be running before type definitions can be generated. Having a file-based approach with glob patterns would really simplify things for frontend authoring, since you wouldn’t need to ensure the server is up and running first.
Relying solely on introspection can also be tricky in some scenarios, for example:
- pipelines that validate whether generated
.d.ts
files were committed - pre-push hooks for the same check
This would be super helpful. Really looking forward to it!
|
||
import type { SchemaLoader, SchemaLoaderResult, OnSchemaUpdate } from './types'; | ||
|
||
const EXTENSIONS = ['.graphql', '.gql', '.json'] as const; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Could this be configurable?
In one of the projects i'm maintaining, since schema is split into multiple files for maintainability purposes, extension is actually .graphqls
. I could imaging people could have even more exotic extensions.
Still unsure if it's worth adding this. Technically, all that'll do may be non-standard, and will work with a small fraction of GraphQL APIs. That's because this isn't the case:
This is basically an assumption. Personally, I haven't used an SDL-based schema/API in years. For a long time, I've also used tagged-template-based GraphQL schemas, rather than SDL-file based ones. The nicest DX currently comes from code-first APIs/schemas (gqtx / pothos / nexus, et al) That means, this is a very limited case. If we add this, the next question we'll get will certainly be around
^This kind of proves my point btw 😅 There's infinite non-standard possibilities to create a GraphQL API that wouldn't be supported by this small addition. I was more interested in adding an example for SDL-on-start functionality, i.e. a small utility that writes the SDL output when the GraphQL server (as long as it's JS-based) starts (or re-starts). Writing the SDL file as the server runs is imho the nicest experience, since it's pretty trivial to commit the SDL file at that point too. |
I ran into a bit of friction here though. I know there’s already support for generating a single schema file and checking it into source control. In our repo though, we already keep the schema under version control, just split across multiple files (since the single file grew too large to be practical). I’d prefer not to also commit the generated schema file, since that can easily lead to merge conflicts. That said, I do see the concern that this setup might be considered non-standard and could lead to a stream of “please support X/Y” requests. One possible compromise could be to allow an arbitrary JS function, maybe as a path to JS file with default export. That way:
|
That's basically what I'd recommend. Since there's no standard (and I don't think there should be), then the responsibility lies with the implementor. If you're using something based on |
Note
Marked as draft since it's kind of a proposal, due to it being requested multiple times. Coordination with
graphqlsp
is also needed.We may also want to use this opportunity to work on error handling (?)
The risk of errors increases by a lot when we accept multiple SDL files, that have potentially not been processed by other tools.
Summary
This has been requested multiple times now, and I thought I'd take a stab at implementing it.
When applied, this PR allows the
schema
option to accept arrays of paths, arrays of globs, and globs. This means that multiple SDL files can now be merged into a single one with thegenerate-schema
command and that the SDL loader will discover multiple files.This has been implemented using
ts.sys.readDirectory
, which should already support this pretty comprehensively.When multiple
.json
files are discovered, an error is thrown.The risk of errors increases by a lot when we accept multiple SDL files, that have potentially not been processed by other tools. This is because they might have been hand-written and not validated by any other tool. I haven't figured out yet what to do about this, especially since GraphQLSP/TSServer cannot output global errors.
I'm currently thinking, we can only merge this if we have a strategy for surfacing global errors in
graphqlsp
.Set of changes
loadFromSDL
config signature has changed to accept arootPath
loadFromSDL
now usests.sys.readDirectory
to discover multiple schema filesloadFromSDL
is used with a single JSON file nothing changes, otherwise, if multiple files are discovered and one is a JSON file, an error is throwngenerate-schema
command now accepts multiple input strings