-
Notifications
You must be signed in to change notification settings - Fork 1.4k
An integration test showing errors with recent alpha version #10565
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: master-next
Are you sure you want to change the base?
An integration test showing errors with recent alpha version #10565
Conversation
🦋 Changeset detectedLatest commit: 900aa10 The changes in this PR will be included in the next version bump. This PR includes changesets to release 1 package
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 |
…config-extractAllFieldsToTypesCompact
| const documentWithExternalFragments: DocumentNode = { | ||
| ...documentNode, | ||
| definitions: [...documentNode.definitions, ...(config.externalFragments || []).map(f => f.node)], | ||
| }; |
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.
I wonder if the functionality in the above statement used to create allFragments could be used for our fragments here somehow? 🤔
It seems to do a similar function of collecting fragments, either from documents or from config.
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.
Good call!
Check the new code - I combined it with allFragments.
dev-test-alpha/.gitignore
Outdated
| @@ -0,0 +1,2 @@ | |||
| **/__generated__/** | |||
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.
I feel we should unignore the generation 🙂
This could be a good test that can be kept as snapshot test, and an advanced use case of codegen
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.
OK, generated files added!
| @@ -0,0 +1,106 @@ | |||
| #!/usr/bin/env ts-node | |||
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.
I created the dev-test-alpha just as a temporary package to demonstrate our company issues.
Do you think it would be worth keeping it?
Then:
- I'll need to add some tests which would get invoked when global tests are running.
- We need to come up with some better name than
dev-test-alpha
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.
Yes, I think so! I find this test is very valuable, as it can be used to catch unexpected bugs, at least in the initial stage after rolling out.
I feel we might be able to simplify a few things in the setup. And worst case, if its usefulness is overshadowed by maintenance, we can decide to drop it (or further simplify) later 🙂
| // @generated | ||
| // This file was automatically generated and should not be edited. | ||
|
|
||
| import type * as Types from '../../graphql-code-generator/dev-test-alpha/src/__generated__/globalTypes'; |
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.
I think we already worked on removing this line in typescript-operations plugin.
But it probably comes from near-operation-file preset.
Shall we ignore it until we do the next version of near-operation-file?
|
|
||
| type Exact<T extends { [key: string]: unknown }> = { [K in keyof T]: T[K] }; | ||
| export type Incremental<T> = T | { [P in keyof T]?: P extends ' $fragmentName' | '__typename' ? T[P] : never }; | ||
| export enum UserManagerRoleType { |
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.
With the recent fix - it adds the enum in the correct file.
Description
This PR includes an integration test which displays issues with the recent version.
Integration tests location:
How to compile:
in the repo root run:
in the dev-test-alpha/ run:
and see that everything compiles
How to run:
in the dev-test-alpha/ run:
Demonstrated issues:
Check the
dev-test-alpha/src/__generated__/*files.Types.prefix should not be included forTypes.UserManagerRoleTypeUserManagerRoleTypedefinition should be included insrc/__generated__/Component.tsfile (like it is included insrc/__generated__/Helper.ts)Update: I've added a potential fix + unit tests for the last issue.