-
Notifications
You must be signed in to change notification settings - Fork 84
feat(formgen): change output directory of model documents in formgen #650
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
feat(formgen): change output directory of model documents in formgen #650
Conversation
🦋 Changeset detectedLatest commit: ac2073c The changes in this PR will be included in the next version bump. This PR includes changesets to release 2 packages
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 |
@@ -98,20 +98,23 @@ export class LocalGraphqlFormGenerator implements GraphqlFormGenerator { | |||
renderTypeDeclarations: true, | |||
apiConfiguration: { | |||
dataApi: 'GraphQL', | |||
fragmentsFilePath: path.join( | |||
fragmentsFilePath: `./${path.join( |
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.
Does this work on windows?
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.
This string is generated straight into the UI Form. If it is left out, the string is sent as graphql/fragments
, which node attempts to resolve as a node module. I can get a windows vm set up to verify this.
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.
Should we get some integration(better)/e2e(worse) test for this where we attempt to use generated code?
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.
This is definitely a missing test vector. I'll get a test written for this.
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.
@@ -251,4 +254,36 @@ void describe('LocalCodegenGraphqlFormGenerator', () => { | |||
const forms = await l.generateForms(); | |||
assert('writeToDirectory' in forms); | |||
}); | |||
const graphqlDirectories = [ |
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.
@alharris-at @sobolk I added these tests which exercise the path generation logic. This is integration-level; it is calling the real codegen package and making assertions against the output.
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.
Issue #, if available:
Description of changes:
The option to customize the output directory of graphql documents used by forms is removed. Graphql documents consumed by generated forms are always generated in a nested graphql directory inside the outDir of the forms.
Additionally, --ui-out-dir has been renamed to --out-dir to maintain consistency with the other commands.
By submitting this pull request, I confirm that you can use, modify, copy, and redistribute this contribution, under the terms of your choice.