Skip to content
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

SimpleSetProps does not emulate SetProps #9

Closed
2 tasks done
tgfisher4 opened this issue Feb 2, 2023 · 4 comments
Closed
2 tasks done

SimpleSetProps does not emulate SetProps #9

tgfisher4 opened this issue Feb 2, 2023 · 4 comments
Labels
bug Something isn't working

Comments

@tgfisher4
Copy link

Guidelines

  • Please search other issues to make sure this bug has not already been reported.

Describe the bug

SetProps is described here as:

Like LowObject & HighObject except that if both keys are defined,
HighObject overrides LowObject instead of intersecting to it

SimpleSetProps is described here as:

Like SetProps<LowObject, HighObject> except it reduces empty {} for
simpler debugging.

I interpret this to mean that SimpleSetProps and SetProps should have the same behavior for all practical purposes, but that the types emitted by SimpleSetProps might be simpler than SetProps.

I observe that SimpleSetProps and SetProps sometimes produce materially different types. This is because SimpleSetProps does not always give precedence to HighObject like SetProps does. In the case that keyof HighObject extends keyof LowObject, SimpleSetProps<LowObject, HighObject> = LowObject. This means that when the keys of HighObject are a nonempty subset of the keys of LowObject, LowObject overrides HighObject for the keys they share, contradicting the behavior of SetProps.

I believe this can be fixed by simply removing the first conditional type from the definition of SimpleSetProps, as shown in the TS bug workbench below.

Steps to reproduce

https://www.typescriptlang.org/dev/bug-workbench/?#code/PTAEG0HsFsEsBcB0ATR8DOBdAFAC3vAA7oBcIA5grgK4BGiAxjMAKa5wMDWAnsNJMhYAnAHYBaYUMhD0wWgBtItYAEYADINosGtABwrdATi0AmAMwqAbCsMM16gCwqArGssAzN+9oPDKsw7IwOhCDMDU8LDysjAIKGjoAMQAMioqYqkmAJQAUMAAVPk5oPmgybCcLKAABgDKLPAAClLEADzJkADuAPK0AFba8AA0oAASsOS4vQMM8AB81aAsAB4MLITwoAigQizI1GvoS9Ab3DUA3gC+i+7SxaXosCfywqCa1OSUIuSI98A5K0I0k28G4hCqtSehBe9SaLXQrWKZS600GS2W8BYImQRyUM2GSPGk1Rs3RmOxuP6gyGOTmoAAvKBKtxIO4xhMplTSStyTimSwWWyOj0ufAkQB+ZEi-FIkj8wVSkmbHlYvnM1ns4miiWazky0CgOXqoUo0WgABk8o1RL1aJVFNAIhYADdhDrhUqLbqlbLQLDmpA2h7RSMbUq5jk8oV7mUKlVqsH8V6w6LFis1htQPBcABDTawNm0SDZ+VHHO7N4sdywJ3IGklGop-GLSCuoRCWCCI4J03NrYidCYnPIUAamuYmSDGvkLOQLZikr-QHArNgiENANtJGJu0Y1WU-H171m+18vHU2kM0DdODwdq96lWtlNwZ0y0v2aRvJgGuCZbxDAcnHYR3BzNYpVec4kQNaBuAAaQFOVBw7b4AG4ckuSNgKEUDwJtSDoNAWCEO4OURGoaAtCEdDMJyUFwVAAA1WAc39eErzYwMEWFYRQw5YQ5nQwQGHkcsqiYAdNmdFjOOIOVmNYjd4XQ6TFLhLjEGIgV0JAA09P00AAD1xUjeiqgUyFnhYWSjkZSzoWspSuPvTpeN1AShO0USKwkwdQFU+yYScuSmJkqEgvU4gVLCqybM0+DtO-AzkoM4yv109BIHkCJYEgEQAPQKMigbcpKhqGyXKVPitXxBZ0QzfNNl2fZDmOU4LmuUBbiEGNHisoRK1oD4vh+P4AWWIEhBBNdQAAQXkCcRDzWBXUCxzIoRbcH25PcHXPWYjw-ZVdrPENL0ZApjWPJNTyOK6d0-A1JQehdDUXJ9FRPE67oFa0OR9J7rsGX17u2zZLSuo6yX3R0XTdQGXuTf7tQNOUKpe6rbVmCM6JmhT5sW5bVvC9bN1suaFuEJbImJ2Lgu4ro3PwoRBJyYTvPEvK-NUgmqaJ6ySZsuVedEfm1vRxmhExjych5ynRZpgW6Y2+KSJ0sAUs1rWDTSpKReplalYcmzQEHKJ5FN8KC1gFgjmzKozKOdwpGgP16f7UBsDzUAXhzQcsizXBbfEv3bdxhiABFYHQPpIHHE27Pp1ormllmkp1kzw6qKOY7jkR4H1sXBfdxlC8V8Wk5T9y090jPTJmgBRABHagc3kBO3Y21pmdTiNa6MzOzNAZvW-kMvDYrjar3H2njaTnvq77jWB6AA

Environment

  System:
    OS: macOS 13.1
    CPU: (10) arm64 Apple M1 Pro
    Memory: 64.67 MB / 32.00 GB
    Shell: 5.8.1 - /bin/zsh
  Binaries:
    Node: 18.12.1 - ~/.nvm/versions/node/v18.12.1/bin/node
    Yarn: 3.2.1 - /Users/Shared/ironclad/git/ironclad_org/ironclad/harbor/node_modules/.bin/yarn
    npm: 8.19.2 - ~/.nvm/versions/node/v18.12.1/bin/npm
  Browsers:
    Chrome: 109.0.5414.119
    Safari: 16.2

Pull request (optional)

  • I can submit a pull request.
@tgfisher4 tgfisher4 added the bug Something isn't working label Feb 2, 2023
@ehmicky
Copy link
Owner

ehmicky commented Feb 4, 2023

Hi @tgfisher4,

You're absolutely correct, great catch!
Thanks a lot for the detailed issue description.

Would you like to submit a PR?
(Link to contributing guidelines)

@ehmicky
Copy link
Owner

ehmicky commented Feb 10, 2023

This should now be fixed with 5.5.1. 🎉

@all-contributors Could you please add @tgfisher4 for bug?

@allcontributors
Copy link
Contributor

@ehmicky

I've put up a pull request to add @tgfisher4! 🎉

@tgfisher4
Copy link
Author

Awesome, sorry I was unresponsive and glad it's now fixed!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

No branches or pull requests

2 participants