hapi
hapi copied to clipboard
Update decorator types to be correct
- The code currently allows any value to be added as a decorator, but the types do not. This work allows any value to be added to a decorator by updating the types to reflect what the code does
- Add relevant tests
- Update inline docs
- Create and export
DecorationValuetype - Fixes #4524
@kanongil when you have a min let me know your thoughts on this. Thanks 🙏
This implementation will remove the typing from an inline method declaration, as X | any will be reduced to any. I believe it should work better if additional declarations for decorate are added with method: any (or maybe even value: any).
Understood. I'll have a look at adding other value specific overloads. I was also wondering on changing the name of the arg to signify that any value can be passed. I'll refactor and push a commit later today
@feedmypixel Agreed with Gil. Perhaps you can type this to be a value from Request | Toolkit | Server interface? I imagine it'd be some utility type that removes internal, reserved keys so that it can pickup whatever you extend via declare module:
https://github.com/hapijs/hapi/blob/master/lib/request.js#L18 https://github.com/hapijs/hapi/blob/master/lib/toolkit.js#L11 https://github.com/hapijs/hapi/blob/master/lib/response.js#L25 https://github.com/hapijs/hapi/blob/master/lib/server.js#L174
Can you also add type tests? This should guarantee that nothing breaks, and you can see it in your editor in the tests if your intended types are producing any, errors, or not. Feel free to add missing tests related to this as well. It's your place to verify your changes.
https://github.com/hapijs/hapi/blob/master/test/types/index.ts
This is a good fix, though. I'm always typing around it with X as never.
@damusix Cheers - will look into this ASAP
@feedmypixel Any updates on this?
@feedmypixel Any updates on this?
Sorry I haven't had time to get to it. I'm hopeful of having some time over the Christmas period
@damusix to pre-empt me getting to this. A few questions:
@feedmypixel Agreed with Gil. Perhaps you can type this to be a value from Request | Toolkit | Server interface? I imagine it'd be some utility type that removes internal, reserved keys so that it can pickup whatever you extend via declare module:
Any thoughts/examples of the type of utility your thinking of?
Can you also add type tests? This should guarantee that nothing breaks, and you can see it in your editor in the tests if your intended types are producing any, errors, or not. Feel free to add missing tests related to this as well. It's your place to verify your changes.
https://github.com/hapijs/hapi/blob/master/test/types/index.ts
No probs 👍
Perhaps this helps: @feedmypixel https://www.typescriptlang.org/play/?#code/C4TwDgpgBAIhDGB7ATgQ2BAcqgttAvFAM7DICWAdgOZQA+xIOARogDYDcAUJ6JFAEoQiEZADcIAE0EBHAK5DgAaQggiUQgApOUHVADkwsSL11tuvbOSsTtMzr1yRIG3f1h0ACxe79eYB8QJb3McMjxg+2FgAFUrCP0ogFkIf0D4vQ8IVAkRInSyINMfPVQwMHSwVlkqSjyi82REWQx01GavevswZAgKnsEiMEQKYTrbYsoAM0R8ogBJCgArBAxC8fMUMioK1DQcMdc9dz2iAEFkNGdOt1QQVkRs9JJ0Xuu9HsHh4XS0AHd0iSIHCoSjpe7bN7gg7FKgQCgiF4DIYjXqcACUXB44GgAFEAB7wCBgYDYPAAHgAKgA+dRQClQCB4jAUCRqAYicRSCCOEjKVRQAD8UHh4mQUAAXHTMZxJrIKPBgGRhlAckg0BgoJSGUy4azYAgUC9SRAqRpeBBJe9ufISHoADRQbqISDIUCS-GE4nGylUh1+AISSUaAB0od2VElqAoIAA2gBdNHqGlRkCJgDeAF9uAB6bN0jxkNSF4WIYBQVBQD4cyRQADWKl+KAknFVhowGitPOA9v0FEQVAAtFCexpE-gaZmMTm8xSC0W1C2DeqIB2el2e3phw7R0moJP2EA
@damusix wonderful thanks 👍🏻
Continued in https://github.com/hapijs/hapi/pull/4538