-
-
Notifications
You must be signed in to change notification settings - Fork 9
feat: user-defined required constructor options #63
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
Conversation
43a90b9 to
61036c4
Compare
|
@JoshuaKGoldberg I think I'm all set from my side, I added the example repository with some code and tests. It's all up for discussion if something is unclear or if you want to suggest a simpler/better implementation |
Co-authored-by: Josh Goldberg <[email protected]>
|
I forgot about moving the tests for the required option into the new example folder, I added a TODO
|
|
@JoshuaKGoldberg are you done from your side? Tests are still failing so I'm not sure if you are still looking into it, happy to review it as is and try to fix the remaining problems |
…tead of a truthy value
|
Currently the IntelliSense in VS Code is happy about this code, but
for the last line 👇🏼 |
|
Ah sorry, I ended up running out of time yesterday. Will tackle more tonight! |
|
no problem at all, I was just checking in |
…hub.com/gr2m/javascript-plugin-architecture-with-typescript-definitions into 60-handle-user-defined-required-options
| [K in keyof Obj]: {} extends Pick<Obj, K> ? undefined : K; | ||
| }[keyof Obj]; | ||
|
|
||
| type RequiredIfRemaining<PredefinedOptions, NowProvided> = | ||
| NonOptionalKeys<RemainingRequirements<PredefinedOptions>> extends undefined |
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.
This was an odd little issue. tsd was giving different issues the different errors than VS Code's intellisense. My best guess for a root cause would be something about different compiler options in tsd' defaults vs. VS Code's when there isn't a tsconfig.json.
I debugged by adding a debugKeys: NonOptionalKeys<RemainingRequirements<PredefinedOptions>>; in the class returned by ConstructorRequiringVersion's new: 32744ac
...which showed itself to be type never in VS Code but type undefined in tsd. 💡
expectType<{ intentionallyFailAlways: true }>(new BaseLevelOne().debugKeys)I switched from never to undefined to make sure both variants of possible compiler options work.
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.
My best guess for a root cause would be something about different compiler options in
tsd'defaults
just fyi, here is tsd's default config:
{ "strict": true, "jsx": "react", "target": "es2017", "lib": ["es2017"], "module": "commonjs", // The following option is set and is not overridable: "moduleResolution": "node" }
https://github.com/SamVerschueren/tsd#custom-typescript-config
|
@gr2m now it's ready 👍 |
gr2m
left a comment
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.
awesome work, thanks for making it work, and for the insights on how you debugged the different behavior between tsd and VS Code's IntelliSense
|
🎉 This PR is included in version 3.4.0 🎉 The release is available on: Your semantic-release bot 📦🚀 |
closes #60
TODOs
examples/required-optionsfolder with a README, code & tests that implements and tests a user-defined required constructor optionsnpm testto run all code & type tests from both the root folderversionrequired option fromBaseand make it dynamic based on user-defined options instead/index.d-test.tsthat test for the requiredversionoption and move them toexamples/required-options/index.test-d.tsinstead