search

ianstormtaylor / superstruct

A simple and composable way to validate data in JavaScript (and TypeScript).
https://docs.superstructjs.org
MIT License
7.02k stars 224 forks source link

Can the minified builds be removed from versions uploaded to npm? #1032

Closed RyanSquared closed 2 years ago

RyanSquared commented 2 years ago

Review of dependencies is important but shipping an (unintentionally, I hope) obfuscated minified file makes it close to impossible to review code. Because of this, and other reasons outlined here: https://gist.github.com/joepie91/04cc8329df231ea3e262dffe3d41f848#why-its-unnecessary-to-include-minified-builds I think it would be beneficial for dependency review purposes to stop shipping a minified source with hopefully minimal impact to others.

ianstormtaylor commented 2 years ago

@RyanSquared I wasn't aware that the minified build is being shipped. Based on package.json it's pointing at the non-minified files in /lib:

https://github.com/ianstormtaylor/superstruct/blob/37dfa752bf198218718ea638f71a9a315931b646/package.json#L10-L14

RyanSquared commented 2 years ago

These lines specify that the package.json should be created with all files in umd and lib:

https://github.com/ianstormtaylor/superstruct/blob/c62e2680cac7ed0035894512c1c24fb970e880a4/package.json#L17-L20

And when inspecting via npm pack superstruct, I can see that there's multiple files listed in package/umd and package/lib:

umd/superstruct.d.ts
umd/types.d.ts
umd/superstruct.min.js
umd/refinements.d.ts
umd/index.d.ts
umd/coercions.d.ts
umd/superstruct.js
umd/superstruct.min.d.ts
umd/utils.d.ts
umd/struct.d.ts
lib/xtras.d.ts
lib/index.cjs.map
lib/types.d.ts
lib/error.d.ts
lib/typings.d.ts
lib/index.es.js.map
lib/refinements.d.ts
lib/utilities.d.ts.map
lib/index.d.ts
lib/coercions.d.ts
lib/xtras.d.ts.map
lib/typings.d.ts.map
lib/index.d.ts.map
lib/refinements.d.ts.map
lib/utils.d.ts.map
lib/utilities.d.ts
lib/error.d.ts.map
lib/index.es.js
lib/utils.d.ts
lib/index.cjs
lib/structs/types.d.ts
lib/structs/refinements.d.ts
lib/structs/utilities.d.ts.map
lib/structs/coercions.d.ts
lib/structs/refinements.d.ts.map
lib/structs/utilities.d.ts
lib/structs/coercions.d.ts.map
lib/structs/types.d.ts.map
lib/coercions.d.ts.map
lib/index.es.d.ts
lib/struct.d.ts
lib/types.d.ts.map
lib/struct.d.ts.map
lib/index.cjs.d.ts

I've also noticed the .map files but those don't include the sourcesContent field which does make them at least partially easier to audit.

ianstormtaylor commented 2 years ago

Gotcha, yeah so you don't need to use that umd/superstruct.min.js file, it's just there for people who want to link directly with a <script> tag. But by default when installing with npm it should be using lib/index.es.js instead.

RyanSquared commented 2 years ago

Right, the package.json specifies that it should load that, but when installing the package with npm it includes all possible files, meaning despite only using one file, I still end up with almost 50.

ianstormtaylor commented 2 years ago

I've added a rm -rf so that the lib and umd directories are cleared out when building, which should remove some of the old artifacts. I believe the rest are in use, mostly typings files since it doesn't seem to combine them into a single export like the source files (open to a PR if this is possible).