fix(npm): include the proper files in the npm tarball #594
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -22,6 +22,9 @@ | |
| } | ||
| } | ||
| }, | ||
| "files": [ | ||
| "lib" | ||
|
There was a problem hiding this comment. Could you include the source Typescript files as well? There was a problem hiding this comment. There is absolutely no reason to do so. Types are provided through the output There was a problem hiding this comment. Also due to the export mapping (a few lines above this) the files in src won't even be accessible. Node will just say they're not exported. The only way to read them would with a manual readFile call. There was a problem hiding this comment. Well, my build system Parcel is capable of using the source files directly. I also prefer untransformed source code that is on GitHub. Given what has been going on with the xz distribution, you should consider this. There was a problem hiding this comment. I don't see any relationship to what happened to xz. It's also not "going on", it's already resolved. Please show me the documentation where parcel says they use the source files. I've used parcel before and it doesn't. Please also explain what your exact use case is that you use parcel to not only consume the git-cliff npm package but also parcel bundle it. Parcel is for bundling webapps after all. There was a problem hiding this comment. I'm not well versed in these topics so I cannot comment. But I would appreciate a ELI5 summary 😊 There was a problem hiding this comment. Sure! @aminya wants me to include the TypeScript files from There was a problem hiding this comment. Optimizing the package size seems unnecessary, and the fact that the popular packages do not provide source files just reveals the wide attack surface in open source. The question remains though. How can I make sure the JavaScript files in the dist are exactly derived from the source TypeScript files in the repository? Do you have a byte-for-byte reproducible build and release process? There was a problem hiding this comment.
This is indeed crucial as it has a direct impact on the download size, and it's important to note that not all users have access to high-speed internet. Moreover, since this also includes a programmatic API, it is not solely installed as a global dependency and it would thus also affect other packages that implement git-cliff, my own favware/cliff-jumper among them. This package is the main reason why I even started contributing to git-cliff in the first place. Furthermore, Yarn Berry (a.k.a. Yarn version 2 and higher) has ceased to support global dependencies.
I understand your concerns. Indeed, you are technically correct. However, it is important to trust the open source community. The recent exploit in one library does not necessarily warrant an Nth degree heightened level of scrutiny. If the issue and my subsequent PR had occurred a few weeks ago, prior to the xz incident, it likely would not have attracted this level of attention. It is not practical to compare the source TS files with the compiled JS files in your node_modules folder for every package. If we were to do this for every package, the process would be extremely time-consuming. Have you considered the number of files in your node_modules folder or the number of transitive dependencies you’re pulling in? This package alone installs a total of 18 packages that would all require analysis. It is crucial to maintain a balanced perspective and not overreact to individual incidents. We should continue to support and have faith in the open source community. To further illustrate the complexity of your proposal, I have attached the tarball that would be published after this PR. Please note that it is zipped because GitHub does not support the upload of Additionally, consider another repository that includes a significant higher amount of code, another project I maintain: sapphiredev/framework. The package can be found here: package.zip There was a problem hiding this comment. Thanks for the detailed summary and your thoughts! I agree that we should still have faith in the open source community. I don't have a strong preference between including/not including the TS files due to the fact that it's an whole another ecosystem that I'm not a part of. I can still very clearly see @aminya's concerns, but I'd say we can discuss this elsewhere for the sake of not blocking this PR. I'm happy to include the TS files if there are strong arguments with practical and proven reproducibility along with a PR. |
||
| ], | ||
| "scripts": { | ||
| "typecheck": "tsc", | ||
| "lint": "eslint .", | ||
|
|
||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
To clarify, what this essentially says is "include only the files in lib, and include everything in there". This ensures that the JavaScript files are also included. Yarn (and npm too for that matter) automatically include the
package.jsonandREADME.mdwhen found, so need to specify them.