fix: add translation behavior to disable gen 1 patterns #2670
Conversation
dpilch
force-pushed
the
allow-gen1
branch
2 times, most recently
from
9d01ea3 to
dee28d6
Compare
| @@ -158,6 +159,14 @@ export class BelongsToTransformer extends TransformerPluginBase { | |||
|
|
|||
| const validate = (config: BelongsToDirectiveConfiguration, ctx: TransformerContextProvider): void => { | |||
| const { field, object } = config; | |||
| if (!ctx.transformParameters.allowGen1Patterns) { | |||
| if (field.type.kind === Kind.NON_NULL_TYPE) { | |||
| throw new InvalidDirectiveError(`@${BelongsToDirective.name} cannot be used on required fields.`); | |||
There was a problem hiding this comment.
nit: Pointing customer to the faulty field by including its name along with parent type name can help them debug faster.
| throw new InvalidDirectiveError(`@${BelongsToDirective.name} cannot be used on required fields.`); | ||
| } | ||
| if (config.fields) { | ||
| throw new InvalidDirectiveError(`fields argument on @${BelongsToDirective.name} is deprecated. Use references instead.`); |
There was a problem hiding this comment.
nit: same suggestion as above about more informative error message. Please apply the same for other Errors added in this PR.
| ).not.toThrow(); | ||
| }); | ||
|
|
||
| test('does not allow @manyToMany', () => { |
There was a problem hiding this comment.
nit here & throughout: test titles should reflect the conditions better, as in
| test('does not allow @manyToMany', () => { | |
| test('does not allow @manyToMany when allowGen1Patterns is false', () => { |
You could accomplish the same thing by organizing nested describe blocks:
describe('allowGen1Patterns', () => {
test('defaults to allow', () => {...});
describe('flag is true', () => {/* all the cases */});
describe('flag is false', () => {/* all the cases */});
});| throw new InvalidDirectiveError(`@${HasManyDirective.name} cannot be used on required fields.`); | ||
| } | ||
| if (config.fields) { | ||
| throw new InvalidDirectiveError(`fields argument on @${HasManyDirective.name} is deprecated. Use references instead.`); |
There was a problem hiding this comment.
Here and throughout all error messages: It's not deprecated, it's outright forbidden in Gen2 projects.
| @@ -25,7 +25,7 @@ import { UserDefinedSlot } from '@aws-amplify/graphql-transformer-core'; | |||
| export const constructTransform: (config: TransformConfig) => GraphQLTransform; | |||
|
|
|||
| // @public (undocumented) | |||
| export const constructTransformerChain: (options?: TransformerFactoryArgs) => TransformerPluginProvider[]; | |||
| export const constructTransformerChain: (options?: TransformerFactoryArgs, allowGen1Patterns?: boolean) => TransformerPluginProvider[]; | |||
There was a problem hiding this comment.
Can we put allowGen1Patterns into TransformerFactoryArgs rather than tacking on a dangling argument?
| @@ -314,6 +314,8 @@ export interface OptimisticConflictResolutionStrategy extends ConflictResolution | |||
| // @public | |||
| export interface PartialTranslationBehavior { | |||
| readonly allowDestructiveGraphqlSchemaUpdates?: boolean; | |||
| // @internal | |||
| readonly _allowGen1Patterns?: boolean; | |||
There was a problem hiding this comment.
JSII requires params marked with @internal to be prefixed with an underscore.
Description of changes
Add translation behavior to disable gen 1 patterns:
@manyToMany@searchable@predictionsfieldsargument on@hasOne,@hasMany, and@belongsTo.@hasOne,@hasMany, and@belongsToon required fields.CDK / CloudFormation Parameters Changed
allowGen1Patternswhich defaults totrue.Issue #, if available
N/A
Description of how you validated changes
Checklist
yarn testpassesBy submitting this pull request, I confirm that my contribution is made under the terms of the Apache 2.0 license.