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

Improve the typescript definitions #356

Closed
43081j opened this issue Oct 15, 2022 · 1 comment · Fixed by #357
Closed

Improve the typescript definitions #356

43081j opened this issue Oct 15, 2022 · 1 comment · Fixed by #357

Comments

@43081j
Copy link
Contributor

43081j commented Oct 15, 2022

Currently many of the exported types are any or very weakly typed.

It would be great if these types could be properly populated via the jsdoc.

For example, the various places you can pass options tend to accept any even though we have a strong type for the user options already.


I did actually start doing this but the overloads you have lead to a lot of craziness in the types.

For example, if we strongly type the overloads of resolve, the tests fail to compile because there are some which explicitly pass in undefined to ensure an error is thrown rather than bailing.

These tests make perfect sense but mean we end up having to dumb down the types to signatures nobody should ever really use. Alternatively we can double-cast undefined via jsdoc (to unknown then to string).

Similarly, the various overloads lead to all sorts of funky types, but are ultimately doable at least.

Maybe if i at least put a draft pr up, you can see what i mean.

cc @sokra maybe if you can list the "officially supported" overloads, that'd help us write the correct types

@43081j
Copy link
Contributor Author

43081j commented Oct 15, 2022

according to the docs, this is fine:

resolve("/some/path/to/folder", "module/dir", fn);

according to the code i then figured it should be impossible to never have a resolve context:

if (!resolveContext)
return callback(new Error("resolveContext argument is not set"));

it turns out the code really is tricked into thinking we have a context, because we just pass the callback twice.

we're really doing resolve(context, path, request, callback, callback), leading the above code to believe we did pass a context.

this kinda stuff is whats making the types pretty difficult to figure out/write.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging a pull request may close this issue.

1 participant