Skip to content

fix(core): generic inference for signal inputs may break with --strictFunctionTypes#54652

Closed
devversion wants to merge 1 commit intoangular:mainfrom
devversion:fix-inference-strict-function-types
Closed

fix(core): generic inference for signal inputs may break with --strictFunctionTypes#54652
devversion wants to merge 1 commit intoangular:mainfrom
devversion:fix-inference-strict-function-types

Conversation

@devversion
Copy link
Member

This commit fixes that the generic inference for signal inputs may break
with --strictFunctionTypes.

We are not using --strictFunctionTypes in all tests, so function parameters are always
checked bivariantly- while in 1P function parameters are checked contravariantly. This breaks
sub-typing and e.g. InputSignal<number> is not a subtype of InputSignalWithTransform<unknown, number>.

This then causes the generic inference via ɵUnwrapDirectiveSignalInputs to always end up resolving
these generic fields to never— breaking inference.

Root cause is that the input signal captures the equality function that is exclusively concerned with the ReadT. And ReadT is never equal, or a supertype of unknown.

https://www.typescriptlang.org/play?#code/KYDwDg9gTgLgBASwHY2FAZgQwMbDgSSTAFcYBlBAcyUwBsB1BGACwBUpMkBndaAWwA8AJWCYAJqwA0celCbBWAPjgBvAFBw4Aei1xWACXxlEXOCzxQIEeNkzEuwAIRmAnmDxhMHPsFRRTXnjYzMDYANbAYnDYECgcAG5eCJwwtC7SAHIA8qxwAEYIALSJcilpAHQacMAAjsR0AFxwABSYTSLiUvntohIAlHAAvMp5VrSiSADcalUA+rNQvaw9ndOa8wDucqjLMtsK0wC+aqCQsIgoaFi4BESkFNR0wkuKVaCoSGKmhCTkVDQMJhsDjcXhQQQdCTSSFKVTHNQxbjwEBNH73f50RgsdicHj8ATEJBhJAQDZIaRIYh8PJoZSDKqU2i0ZyYb53P6PWgCSnU2nTKpaABUM00rDceAA5GiOQDuVSaVBFBKTHASfBWVwMXlxmYIK53HApeyHgCscDcWDBITiaTyar5bSJZVReLTBB0HAwJZ3LAXIbavVaMrAhcYnxPDAENrgM7NHpxYbWk0eQrpHlkw6oANhvkxhNlQhTGq4BqtTqYHqYAmJUm4NaSWS00167bsyM85wnVU4-H3G6PZ5vL40KYJZhg59DeOS4tQxBw5hI9HYz2XQaJS2yQWi9YS1xNY9o7r9ZKU2gnc0AEwzDTZVgAUSaABF7wAFe8ZF8ZXJZDJwAAGhSFFwMByNgMAAGKEuBCCxGKfb-nA6C0JglArqugpaEAA

@devversion devversion added action: review The PR is still awaiting reviews from at least one requested reviewer target: patch This PR is targeted for the next patch release labels Feb 28, 2024
@devversion devversion requested a review from crisbeto February 28, 2024 16:09
Copy link
Member

@crisbeto crisbeto left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can we capture this in a test somehow?

@devversion
Copy link
Member Author

The new tests for --strict do capture this. Are you thinking of something else?

…ctFunctionTypes`

This commit fixes that the generic inference for signal inputs may break
with `--strictFunctionTypes`.

We are not using --strictFunctionTypes` in all tests, so function parameters are always
checked bivariantly- while in 1P function parameters are checked contravariantly. This
breaks sub-typing and e.g. `InputSignal<number>` is not a subtype of `InputSignalWithTransform<unknown, number>`.

Root cause is that the input signal captures the equality function that
is exclusively concerned with the `ReadT`. And `ReadT` is never equal, or a
supertype of unknown.

https://www.typescriptlang.org/play?#code/KYDwDg9gTgLgBASwHY2FAZgQwMbDgSSTAFcYBlBAcyUwBsB1BGACwBUpMkBndaAWwA8AJWCYAJqwA0celCbBWAPjgBvAFBw4Aei1xWACXxlEXOCzxQIEeNkzEuwAIRmAnmDxhMHPsFRRTXnjYzMDYANbAYnDYECgcAG5eCJwwtC7SAHIA8qxwAEYIALSJcilpAHQacMAAjsR0AFxwABSYTSLiUvntohIAlHAAvMp5VrSiSADcalUA+rNQvaw9ndOa8wDucqjLMtsK0wC+aqCQsIgoaFi4BESkFNR0wkuKVaCoSGKmhCTkVDQMJhsDjcXhQQQdCTSSFKVTHNQxbjwEBNH73f50RgsdicHj8ATEJBhJAQDZIaRIYh8PJoZSDKqU2i0ZyYb53P6PWgCSnU2nTKpaABUM00rDceAA5GiOQDuVSaVBFBKTHASfBWVwMXlxmYIK53HApeyHgCscDcWDBITiaTyar5bSJZVReLTBB0HAwJZ3LAXIbavVaMrAhcYnxPDAENrgM7NHpxYbWk0eQrpHlkw6oANhvkxhNlQhTGq4BqtTqYHqYAmJUm4NaSWS00167bsyM85wnVU4-H3G6PZ5vL40KYJZhg59DeOS4tQxBw5hI9HYz2XQaJS2yQWi9YS1xNY9o7r9ZKU2gnc0AEwzDTZVgAUSaABF7wAFe8ZF8ZXJZDJwAAGhSFFwMByNgMAAGKEuBCCxGKfb-nA6C0JglArqugpaEAA
@devversion devversion force-pushed the fix-inference-strict-function-types branch from b704e90 to 42a743c Compare February 28, 2024 17:27
@pkozlowski-opensource pkozlowski-opensource added the area: core Issues related to the framework runtime label Feb 28, 2024
@ngbot ngbot bot added this to the Backlog milestone Feb 28, 2024
@devversion devversion added action: merge The PR is ready for merge by the caretaker and removed action: review The PR is still awaiting reviews from at least one requested reviewer labels Feb 29, 2024
@pkozlowski-opensource
Copy link
Member

This PR was merged into the repository by commit 78e6911.

pkozlowski-opensource pushed a commit that referenced this pull request Mar 4, 2024
…ctFunctionTypes` (#54652)

This commit fixes that the generic inference for signal inputs may break
with `--strictFunctionTypes`.

We are not using --strictFunctionTypes` in all tests, so function parameters are always
checked bivariantly- while in 1P function parameters are checked contravariantly. This
breaks sub-typing and e.g. `InputSignal<number>` is not a subtype of `InputSignalWithTransform<unknown, number>`.

Root cause is that the input signal captures the equality function that
is exclusively concerned with the `ReadT`. And `ReadT` is never equal, or a
supertype of unknown.

https://www.typescriptlang.org/play?#code/KYDwDg9gTgLgBASwHY2FAZgQwMbDgSSTAFcYBlBAcyUwBsB1BGACwBUpMkBndaAWwA8AJWCYAJqwA0celCbBWAPjgBvAFBw4Aei1xWACXxlEXOCzxQIEeNkzEuwAIRmAnmDxhMHPsFRRTXnjYzMDYANbAYnDYECgcAG5eCJwwtC7SAHIA8qxwAEYIALSJcilpAHQacMAAjsR0AFxwABSYTSLiUvntohIAlHAAvMp5VrSiSADcalUA+rNQvaw9ndOa8wDucqjLMtsK0wC+aqCQsIgoaFi4BESkFNR0wkuKVaCoSGKmhCTkVDQMJhsDjcXhQQQdCTSSFKVTHNQxbjwEBNH73f50RgsdicHj8ATEJBhJAQDZIaRIYh8PJoZSDKqU2i0ZyYb53P6PWgCSnU2nTKpaABUM00rDceAA5GiOQDuVSaVBFBKTHASfBWVwMXlxmYIK53HApeyHgCscDcWDBITiaTyar5bSJZVReLTBB0HAwJZ3LAXIbavVaMrAhcYnxPDAENrgM7NHpxYbWk0eQrpHlkw6oANhvkxhNlQhTGq4BqtTqYHqYAmJUm4NaSWS00167bsyM85wnVU4-H3G6PZ5vL40KYJZhg59DeOS4tQxBw5hI9HYz2XQaJS2yQWi9YS1xNY9o7r9ZKU2gnc0AEwzDTZVgAUSaABF7wAFe8ZF8ZXJZDJwAAGhSFFwMByNgMAAGKEuBCCxGKfb-nA6C0JglArqugpaEAA

PR Close #54652
@angular-automatic-lock-bot
Copy link

This issue has been automatically locked due to inactivity.
Please file a new issue if you are encountering a similar or related problem.

Read more about our automatic conversation locking policy.

This action has been performed automatically by a bot.

@angular-automatic-lock-bot angular-automatic-lock-bot bot locked and limited conversation to collaborators Apr 4, 2024
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

action: merge The PR is ready for merge by the caretaker area: core Issues related to the framework runtime target: patch This PR is targeted for the next patch release

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants