search

adaltas / node-csv

Full featured CSV parser with simple api and tested against large datasets.
https://csv.js.org
MIT License
3.98k stars 263 forks source link

fix: types weren't working for commonjs. Run tsc and lint to validate changes #397

Closed DavidTanner closed 1 year ago

DavidTanner commented 1 year ago

When requiring the package, like import { stringify } from 'csv-stringify'; Typescript is complaining that ./index.js cannot be found. Converting the import line in the .d.cts file to ./index.cjs fixes the issue.

To avoid future publishing issues around typescript, and to validate the change, I added a tsc command to run after linting, and run linting before tests.

I also fixed linting issues that were uncovered.

wdavidw commented 1 year ago

Thank you for your contribution. One requested change, the sed command did not work for me, instead I had to use "find dist/cjs -name '*.d.cts' -exec sed -i \"s/\\.js'/\\.cjs'/g\" {} \\;", (no '' after sed -i).

wdavidw commented 1 year ago

Strange, why did you used sh, the suggestion above did not work for you ?

DavidTanner commented 1 year ago

Correct, in BSD the -i requires a '' after it. So to get it to work on linux and MacOS, I need it to try both ways.

https://github.com/lmquang/til/issues/18

wdavidw commented 1 year ago

Strange because I am on macos and it didn't work with -i ''

DavidTanner commented 1 year ago

Could be the sed version you have. If you used brew to install the gnu version of sed then it wouldn't require the ''.

wdavidw commented 1 year ago

Exact, in my case it was installed by nix (nix-darwin to be exact).

DavidTanner commented 1 year ago

So you are running a nix version of sed, and I am running BSD. What do I need to do to get this change in for the types?

wdavidw commented 1 year ago

Let merge it this way, thank you for your contribution