ryu1kn / csv-writer

Convert objects/arrays into a CSV string or write them into a CSV file
https://www.npmjs.com/package/csv-writer
MIT License
246 stars 39 forks source link

Typings should point to .d.ts files instead of .ts files #54

Closed walkerburgin closed 3 years ago

walkerburgin commented 4 years ago

I noticed in package.json that the types field is pointing to the TypeScript source files instead of the .d.ts files. In practice this means that consumers are basically re-compiling csv-writer, which is more brittle than it needs to be.

ryu1kn commented 3 years ago

csv-writer is publishing the transpiled js files and main entry point of this library is also js; so the consumer doesn't need to compile the bundled .ts files to use the library. For example, the integration test is a pure js project with no transpilation involved 👉 test-integration/test.sh

Having said that, including the whole .ts file in the published package is unnecessary, and we can just include .d.ts instead as you suggested.

ryu1kn commented 3 years ago

Hmm... forgot that I did this way (include .ts instead of .d.ts) along with source map files to provide better debugging experience.

Can't say the 2nd option is better than the 1st. I'll leave it like this for now. Thanks for bringing this up though!

walkerburgin commented 3 years ago

Can I convince you to reconsider? You're right that consumers don't need to re-transpile from TypeScript to JavaScript in order to run csv-writer, but they do effectively need to recompile it in order to type check a library which depends on csv-writer (without skipLibCheck). If you look at the TypeScript documentation here they recommend setting types/typings specifically to the bundled declaration file (.d.ts). If you look at TypeScript itself as an example, they set typings to a .d.ts file as well (here). Other popular libraries written in TypeScript like redux do the same (here).

If you want to publish the original source for easier debugging, I think the right way to do that is to generate and include source maps and declaration maps (using the --declarationMap flag) as well as the original .ts files. So you'd include .js/.ts/.d.ts/.d.ts.map in the published package, point main to the .js file and types to the .d.ts file.

JasonKleban commented 3 years ago

I suspect this is causing these errors in my library which itself publishes its own .d.ts files and is having trouble typechecking csv-writer in the process - even though no value or type is getting reexported, nor accepted nor returned by any function.

image

TS2742: The inferred type of 'createArrayCsvStringifier' cannot be named without a reference to ...