Repository navigation
fix(ivy): validate props and attrs with "on" prefix at runtime - #28054
AndrewKushnir wants to merge 2 commits into
Conversation
AndrewKushnir
added
type: bug/fix
action: review
|
You can preview fbec37a at https://pr28054-fbec37a.ngbuilds.io/. |
|
You can preview b6f6117 at https://pr28054-b6f6117.ngbuilds.io/. |
There was a problem hiding this comment.
nit: consider removing double negation (rename to sth like validateAttrOrProp and remove !). While we are at the names maybe we should be explicit what the validation is all about. validateName?
pkozlowski-opensource
left a comment
pkozlowski-opensource
left a comment
There was a problem hiding this comment.
LGTM.
Left one nit, feel free to ignore.
pkozlowski-opensource
added
action: merge
Prior to this change we performed prop and attr name validation at compile time, which failed in case a given prop/attr is an input to a Directive (thus should not be a subject to this check). Since Directive matching in Ivy happens at runtime, the corresponding checks are now moved to runtime as well.
b6f6117 to
8535a52
Compare
|
You can preview 8535a52 at https://pr28054-8535a52.ngbuilds.io/. |
|
This issue has been automatically locked due to inactivity. Read more about our automatic conversation locking policy. This action has been performed automatically by a bot. |
Prior to this change we performed prop and attr name validation at compile time, which failed in case a given prop/attr is an input to a Directive (thus should not be a subject to this check). Since Directive matching in Ivy happens at runtime, the corresponding checks are now moved to runtime as well.
Messages in
validatePropertyandvalidateAttribute functionsfunctions were taken from the corresponding functions in Compiler, to make sure all asserts remain the same.This PR resolves FW-786.
PR Type
What kind of change does this PR introduce?
Does this PR introduce a breaking change?