palantir / plottable

:bar_chart: A library of modular chart components built on D3
http://plottablejs.org/
MIT License
2.98k stars 221 forks source link

Include the source code in npm package #3347

Open chenzhutian opened 7 years ago

chenzhutian commented 7 years ago

Hi, thanks for your greate work. I'm a typescript user and use webpack to bundle my project files. The webpack@2 has the Tree Shaking features which can reduce the size of files. Since this feature can only work on es6, can you include the source code (the ts code are ok because I can use ts-loader to convert them), other than the production code in the npm package?

adidahiya commented 7 years ago

related: https://github.com/palantir/blueprint/issues/309

In blueprint, I suggested publishing ES6 / ES2015 modules in the distribution; I think plottable should do the same. You can use ts-loader to bundle the library files in either case (.ts vs ES6 .js), but I think typings resolution will be simpler if the distribution includes .d.ts and .js files.

chenzhutian commented 7 years ago

I think both including the ts source code and publishing as es6 are OK. Is there any similar plan for portable?

hellochar commented 7 years ago

Thanks for the report @chenzhutian , sounds reasonable. This should be an easy external PR - we can simply whitelist src/ in the .npmignore to include the TS. We'll be happy to accept it!

adidahiya commented 7 years ago

Please verify that the compiler is able to handle .ts files inside node_modules before making this change. Make sure to test TS 2.1, 2.2, and 2.3.

chenzhutian commented 7 years ago

After some researching, according to here https://github.com/Microsoft/TypeScript/issues/12358, I think the office team does not recommend to publish .ts source code for some reasons. It seems that publish the es6 code is a better choice.