feat: Sync codegen behavior implementation - modify and integrate #892
Conversation
stocaaro
force-pushed
the
02-sync-codegen
branch
from
c03deaa
to
e5c11a1
Compare
| @@ -25,7 +25,7 @@ | |||
| "@aws-amplify/graphql-docs-generator": "4.2.1", | |||
| "@aws-amplify/graphql-generator": "0.4.6", | |||
| "@aws-amplify/graphql-types-generator": "3.6.0", | |||
| "@graphql-codegen/core": "2.6.6", | |||
| "@graphql-codegen/core": "^2.6.6", | |||
There was a problem hiding this comment.
We had multiple versions in our node_modules, this consolidates down to a single version which makes it easier to detect if we upgrade.
| "watch": "tsc -w", | ||
| "test": "jest", | ||
| "test-watch": "jest --watch", | ||
| "extract-api": "ts-node ../../scripts/extract-api.ts" | ||
| "extract-api": "ts-node ../../scripts/extract-api.ts", | ||
| "check-codegen-version": "bash ./scripts/check-codegen-version.sh" |
There was a problem hiding this comment.
Adding a command that makes sure our vendor code matches our dependency during the build process
| For the code forked here most of the complexity of adapting `codegen` to be non-async is captured in type changes surfaced by `@aws-amplify/appsync-modelgen-plugin` as `SyncTypes`. If these base types change when we update the package, our derived types to support Sync execution may need to change as well | ||
|
|
||
| These are the steps needed to adapt the `graphql-codegen-core` code to run non-async and adhere to the adapted `SyncTypes`: |
There was a problem hiding this comment.
Following these instructions with the current target version will result in the same diff proposed with this change give or take minor details or points where discretion is involved (such as how to handle resolving options).
| export type SyncPluginMap<Obj extends PluginMapContainer> = Omit<Obj, 'pluginMap'> & { | ||
| pluginMap: { | ||
| [name: string]: Omit<Obj['pluginMap'][string], 'plugin'> & { | ||
| plugin: ( | ||
| ...args: Parameters<Obj['pluginMap'][string]['plugin']> | ||
| ) => Awaited<ReturnType<Obj['pluginMap'][string]['plugin']>>; | ||
| }; | ||
| }; | ||
| }; | ||
|
|
||
| export type SyncCache<Obj extends CacheContainer> = Omit<Obj, 'cache'> & { | ||
| cache?: (<T>(namespace: string, key: string, factory: () => T) => T) | undefined | ||
| }; |
There was a problem hiding this comment.
These two helper types do most of the lifting here. They unwrap the Promise/Await type expectations so that the types can be used for sync versions of the target behavior.
Awaited behavior is used/allowed for:
- Plugin execution
- Profilers
- Caches
| test(`basic ${target}`, async () => { | ||
| const generators = ['generateModels', 'generateModelsSync'] as const; | ||
|
|
||
| async function runGenerator(generator: (typeof generators)[number], options: GenerateModelsOptions) { |
There was a problem hiding this comment.
Rather than have two copies of the tests, here we use a string to run th same tests for both the sync and async versions.
| test('does not fail on custom directives', async () => { | ||
| const targets: ModelsTarget[] = ['java', 'swift', 'javascript', 'typescript', 'dart', 'introspection']; | ||
| targets.forEach(target => { | ||
| test(`both generates generate the same basic ${target}`, async () => { |
There was a problem hiding this comment.
Make sure the sync/async behavior is the same and remains the same.
Description of changes
This change translates the vendored code for codegen, adapting it so that it isn't async. Steps to adapt the code have been made as simple as possible where complexity is captured in related code to make upgrades as easy as possible as we maintain this. Directions are documented.
@graphql-codegen/coreforkCommits
@graphql-codegen/corefile acrosscodegenSyncbehaviorCodegen Paramaters Changed or Added
Adding
Description of how you validated changes
Checklist
yarn testpassespath) behave differently on windows.type,input,enum,interface,unionand scalar types.By submitting this pull request, I confirm that my contribution is made under the terms of the Apache 2.0 license.