This repository was archived by the owner on Dec 25, 2025. It is now read-only.
Fix invalid schema and typos in graphql-config - #54
Merged
Conversation
commit: |
IzumiSy
force-pushed
the
fix_graphql-config_exports
branch
from
October 28, 2024 09:16
fdfe10d to
a616d99
Compare
IzumiSy
commented
Oct 28, 2024
Comment on lines
-7
to
-10
| const mergeDocumentNodes = (docs: DocumentNode[]) => ({ | ||
| kind: Kind.DOCUMENT, | ||
| definitions: docs.flatMap((doc) => doc.definitions), | ||
| }); |
Member
Author
There was a problem hiding this comment.
Found that merging like this is not working as expected, so now here is using use @graphql-tools/merge instead.
IzumiSy
force-pushed
the
fix_graphql-config_exports
branch
from
October 28, 2024 09:49
a883c4d to
90af572
Compare
IzumiSy
marked this pull request as draft
October 28, 2024 10:03
IzumiSy
commented
Oct 29, 2024
| import { buildASTSchema } from "graphql"; | ||
| import { schemaDefinition } from "./schema"; | ||
|
|
||
| describe.skip("schemaDefinition", () => { |
Member
Author
There was a problem hiding this comment.
I am going to remove skip here when I work on #56.
IzumiSy
marked this pull request as ready for review
October 29, 2024 10:15
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to subscribe to this conversation on GitHub.
Already have an account?
Sign in.
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
This PR has the following two bugfixes.
Bug 1: types in exports
I came across unexpected behaviour that apps that use
@fabrix-framework/graphql-configcannot import anything.That is caused by typos in packge.json exports made in #32. This PR fixes them.
Bug 2: custom schema definitions go
undefinedThis is bug caused in #42 which has broken up a custom graphql schema into multiple files, and it accidentally removed a line to call
parsefunction from graphql-js package. GraphQL files are loaded as string, so parsing it intoDocumentNodeis required. However, I could not notice it at that time due to usage ofvite-plugin-graphql-loaderin tests that dazzles TypeScript compiler, so on production the GraphQL files that I tried to import gets undefined.In order to prevent the issue, I removed vite-plugin-graphql-loader and just use vite-plugin-string to make it work as the same condition as tsup does to build for production.
Plus, I added a test that checks if the schema is valid or not with
buildASTSchemafunction from graphql-js.Leftover: invalid directive definitions
By making CI work as expected, I discovered that constraint feature has several duplicated fields that are not allowed in GraphQL, but fixing that would make more changes in core, so I would like to isolate the topic and limit the changes in this PR just in graphql-config package if possible.
#56 is the issue I have created to work on in another PR, maybe next of this.