-
Notifications
You must be signed in to change notification settings - Fork 26
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
Check for additionalProperties to be set to false #87
Conversation
@@ -76,6 +76,7 @@ export type BlockActionsBody = { | |||
id: string; | |||
/** | |||
* @description User's handle as seen in the Slack client when e.g. at-notifying the user. | |||
*/ |
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.
There was an issue here where we weren't closing the comment, which meant that name
wasn't part of the BlockActionsBody defined payload. This is unrelated to the core of this PR and can be removed if we want.
Codecov Report
@@ Coverage Diff @@
## main #87 +/- ##
=======================================
Coverage 96.10% 96.10%
=======================================
Files 41 41
Lines 1565 1565
Branches 87 87
=======================================
Hits 1504 1504
Misses 59 59
Partials 2 2
📣 We’re building smart automated test selection to slash your CI/CD build times. Learn more |
assertExists(result.outputs?.noAddlPropertiesObj); | ||
assertExists(result.outputs?.noAddlPropertiesObj.aString); | ||
|
||
// @ts-expect-error anythingElse cant exist |
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'm not sure there's a better way to do this type of test, but if this directive becomes unused (i.e. no error), it will let us know!
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.
TIL
& { | ||
[k in keyof Param["properties"]]: FunctionInputRuntimeType< | ||
Param["properties"][k] | ||
>; | ||
} |
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.
We could probably figure out some way to avoid this duplication, but I didn't want to spend too much time on this. Open to suggestions.
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.
beautiful, works as described.
assertExists(result.outputs?.noAddlPropertiesObj); | ||
assertExists(result.outputs?.noAddlPropertiesObj.aString); | ||
|
||
// @ts-expect-error anythingElse cant exist |
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.
TIL
Summary
The backend defaults
additionalProperties
on parameters of theobject
type to true, so this PR inverts the check to see if they are explicitly settingadditionalProperties
to false.Testing
object
parameter as a function input or outputadditionalProperties: false
Requirements (place an
x
in each[ ]
)