search

octet-stream / form-data

Spec-compliant FormData implementation for Node.js
https://www.npmjs.com/package/formdata-node
MIT License
142 stars 17 forks source link

Fix package exports for TypeScript #48

Closed jaydenseric closed 2 years ago

jaydenseric commented 2 years ago

This PR fixes the package exports field for compatibility with prerelease typescript v4.6. Weirdly, tsc in the CLI didn't have problems resolving the type definition for imports but the VS Code TypeScript extension did.

Project code before, with typescript v4.6.0-dev.20211227:

Screen Shot 2021-12-28 at 11 05 07 am

After the changes in this PR:

Screen Shot 2021-12-28 at 11 06 28 am

The specific change needed for the fix in the package exports field was the addition of exports["."].node.types. The others are added because it might solve other problems. I don't fully understand what the point of the typesVersions field is, so you might want to consider it.

See this TypeScript guide.

For the record, this PR works with the patterns already established but I would setup the files and package exports differently.

codecov[bot] commented 2 years ago

Codecov Report

Merging #48 (36e5c8d) into master (2376841) will not change coverage. The diff coverage is n/a.

Impacted file tree graph

@@            Coverage Diff            @@
##            master       #48   +/-   ##
=========================================
  Coverage   100.00%   100.00%           
=========================================
  Files            9         9           
  Lines          964       964           
  Branches       137       137           
=========================================
  Hits           964       964
Flag Coverage Δ
unittests 100.00% <ø> (ø)

Flags with carried forward coverage won't be shown. Click here to find out more.

Continue to review full report at Codecov.

Legend - Click here to learn more Δ = absolute <relative> (impact), ø = not affected, ? = missing data Powered by Codecov. Last update 2376841...36e5c8d. Read the comment docs.

octet-stream commented 2 years ago

I don't fully understand what the point of the typesVersions field is, so you might want to consider it.

It was introduced as the way to tell TS where to pick up the types for file-from-path, I didn't find another way before and it looks like your PR is introducing the one. So, typesVersion will be removed in a future, but as for now I would rather keep it. It's weird that TS now can't pick up types from types field. I don't really understand what is going on there and why they can't just pick up what I declared in this field.

For the record, this PR works with the patterns already established but I would setup the files and package exports differently.

What do you mean?

jaydenseric commented 2 years ago

What do you mean?

Sorry to tease, but it would be perhaps a 6+ paragraph comment to adequately explain all the nuances and I'm a bit burned out from high-effort comments at the moment.

octet-stream commented 2 years ago

It's fine. Take your time, if you would comment on this in a future. By the way, I'm planning to drop CJS support for this project by the Node.js v12 EOL, so I might need to do something about exports field anyway. Will see.

octet-stream commented 2 years ago

Everything looks good so far, but I can't release this immediately because I am busy on my work. Sorry. I'll merge it and include with the next release as soon as I can.

octet-stream commented 2 years ago

Sorry for the delay :) This change is now available in https://github.com/octet-stream/form-data/releases/tag/v4.3.2