X Tutup
The Wayback Machine - https://web.archive.org/web/20210410015043/https://github.com/nodejs/node/pull/38182
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

lib: add typings for fileURLToPath #38182

Draft
wants to merge 2 commits into
base: master
from
Draft

Conversation

@bmeck
Copy link
Member

@bmeck bmeck commented Apr 10, 2021

This types internal/url.js fileURLToPath.

It actually adds a typings to 3 locations:

  • getPathFromURLWin32 to specify the parameter it takes
  • URL.prototype.pathname to make inference work on the return type of getPathFromURLWin32
  • fileURLToPath to specify the parameters it takes

I did not actually specify the return types while doing my initial check of these since that would be determined by the implementation details and specifying the return value might not match the implementation. Due to the desire to keep checks turned off by default due to the overall lack of typings at this point in the project. Be sure to enable checks by changing :

"checkJs": false,

checkJs to true in node/tsconfig.json and being sure to restart any TS Server you might have running in your IDE to perform checks.

After enabling checks I set the expected return types and then validated that there were no errors before changing checkJs back to false and pushing out the PR.

There are no tests for such typings, but you can show proof of your work by attaching simple images which could then be validated by a reviewer:

  • before

Screen Shot 2021-04-09 at 20 47 59

*after
Screen Shot 2021-04-09 at 20 47 39

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

Successfully merging this pull request may close these issues.

None yet

2 participants
X Tutup