Show warning when unknown stack-args properties are added

[jpb]

Example output:

warn Invalid property 'ServiceRoleArn' in a.yaml. Did you mean 'ServiceRoleARN'?
warn Invalid property 'foo' in a.yaml

Fixes #131

[tavisrudd]

Any thoughts about how to ensure we keep this in sync?

[jpb]

The type (Array<keyof StackArgs>) should help – I wasn't able to find another instance of this list to pull it from (the StackArgs is only a type and so the keys can't be pulled off of it).

[tavisrudd]

What about $merge if someone were to use that at the top level?

[jpb]

I thought about that and I agree that it should be supported – I debated moving the warnings check until after transform is called, but mistyping something like AssumeRoleARN or Profile could result in fetching imports to fail and it would not be obvious that the parameter name is the culprit.

I think that even though it is less helpful, it probably makes more sense to move it until after transform.

[tavisrudd]

What about just skipping anything starting with $?

…

On Mon, Nov 19, 2018 at 5:37 PM James Brennan ***@***.***> wrote: ***@***.**** commented on this pull request. ------------------------------ In src/cfn/index.ts <#152 (comment)>: > + 'OnFailure', + 'Parameters', + 'Profile', + 'Region', + 'ResourceTypes', + 'RoleARN', + 'ServiceRoleARN', + 'StackName', + 'StackPolicy', + 'Tags', + 'Template', + 'TimeoutInMinutes', + 'UsePreviousTemplate', +]; + +const stackArgsMetaProperties = ['$imports', '$defs']; I thought about that and I agree that it should be supported – I debated moving the warnings check until after transform is called, but mistyping something like AssumeRoleARN or Profile could result in fetching imports to fail and it would not be obvious that the parameter name is the culprit. I think that even though it is less helpful, it probably makes more sense to move it until after transform. — You are receiving this because your review was requested. Reply to this email directly, view it on GitHub <#152 (comment)>, or mute the thread <https://github.com/notifications/unsubscribe-auth/AAOdVo8S3qQfcqOYsifsjIy40r6VfZdRks5uw1zjgaJpZM4YqKNc> .

[jpb]

I ended up using

iidy/src/preprocess/index.ts

Line 102 in d0e627a

const extendedCfnDocKeys = ['$imports', '$defs', '$params', '$location', '$envValues'];

which works well working on the data post transform.

[tavisrudd]

Looks good. Merge away. Let's rebase #150 (and its dependent PRs) after.

[tavisrudd]

Good to go after rebased and conflicts resolved.