toss / es-toolkit

A modern JavaScript utility library that's 2-3 times faster and up to 97% smaller—a major upgrade to lodash.
https://es-toolkit.slash.page
Other
6.17k stars 253 forks source link

Duplication of TypeScript sources in sourcemaps #285

Closed fvsch closed 1 month ago

fvsch commented 1 month ago

(I’m extracting this question from #255 to keep the scope of that PR manageable.)

The current build of this library with tsup, and the proposed build config with Rollup in #255, produce an output where the TypeScript sources of this library are copied 3 times in published sourcemaps.

For example, let's take the contents of the source file src/array/difference.ts.

With the current tsup build config, they end up being copied into the sourcesContent of 8 sourcemaps:

dist/browser.js.map           (CJS sourcemap)
dist/chunk-JPUKH67K.mjs.map   (ESM sourcemap)
dist/index.js.map             (CJS sourcemap)
dist/array/difference.js.map  (CJS sourcemap)
dist/array/index.js.map       (CJS sourcemap)
dist/array/xor.js.map         (CJS sourcemap)
dist/compat/index.js.map      (CJS sourcemap)
umd/browser.global.js.map     (IIFE sourcemap)

(The CommonJS build is a big culprit here. This duplication internal to the CJS build is fixed in #255.)

With the proposed Rollup config in #255, this duplication is down to 3 files:

dist/array/index.js.map        (CJS sourcemap)
dist/array/difference.mjs.map  (ESM sourcemap)
umd/browser.global.js.map      (IIFE sourcemap)

But it's still three copies of the source TypeScript file.

I'm opening this issue to check that this duplication is okay, and ask whether we want to eliminate this duplication by publishing the TS files in the package.

It should be possible to publish the src/**/*.ts files (adding src/**/*.spec.ts to .npmignore), and set TypeScript's inlineSources option to false when building, leaving individual source maps looking like this:

{
  "version": 3,
  "sources": ["../../src/array/difference.ts"],
  "sourcesContent": [],
  "mappings": ";;;;;;;;;;;;;;;;;;;;AAAA;AAAA;AAAA;AAAA;AAAA;AAsBO,SAAS,WAAc,UAAwB,WAA8B;AAClF,QAAM,YAAY,IAAI,IAAI,SAAS;AAEnC,SAAO,SAAS,OAAO,UAAQ,CAAC,UAAU,IAAI,IAAI,CAAC;AACrD;",
  "names": []
}

vs the current output which looks like:

{
  "version": 3,
  "sources": ["../../src/array/difference.ts"],
  "sourcesContent": [
    "/**\n * Computes the difference ... firstArr.filter(item => !secondSet.has(item));\n}\n"
  ],
  "mappings": ";;;;;;;;;;;;;;;;;;;;AAAA;AAAA;AAAA;AAAA;AAAA;AAsBO,SAAS,WAAc,UAAwB,WAA8B;AAClF,QAAM,YAAY,IAAI,IAAI,SAAS;AAEnC,SAAO,SAAS,OAAO,UAAQ,CAAC,UAAU,IAAI,IAAI,CAAC;AACrD;",
  "names": []
}

Possible impacts:

Personally, I don't have a strong opinion either way. (For a library like this I would tend to not publish sourcemaps at all in the first place, as long as I’m publishing unminified code that is readable. But that's just a personal preference.)

raon0211 commented 1 month ago

As you mentioned, as long as our library is human readable, we might not need source maps at all. So I guess we might just skip shipping source maps with our library.

If we should add source maps, I guess we should ship TypeScript sources and use the sources property only. Seems that it complies with the source map specification when I check.

fvsch commented 1 month ago

So I looked into this a bit more deeply, and tried different configurations.

Sourcemaps strategy

For each of the ESM/CJS and browser builds, we can:

  1. Not generate sourcemaps at all
  2. Output sourcemaps with inlineSources: true (original sources are copied in the .map files)
  3. Output sourcemaps with inlineSources: false (in which case we need to publish original sources)

Turning those options on or off for the different builds gives different results when it comes to package size. The current strategy is highlighted with bold file sizes.

ESM/CJS sourcemaps Browser sourcemap Publish src .tgz size .tar size
📄 Inline sources 📄 Inline sources 🚫 No 179,599 1,512,448
🔗 External sources 📄 Inline sources ✅ Yes 177,387 1,482,240
🔗 External sources 🔗 External sources ✅ Yes 146,010 1,321,472
🚫 No sourcemap 🔗 External sources ✅ Yes 118,399 1,090,048
🚫 No sourcemap 📄 Inline sources 🚫 No 110,336 955,392
🚫 No sourcemap 🚫 No sourcemap 🚫 No 68,299 764,416

Looking at these results, external sources are a small win for package size, but the effect is not huge.

There's also an issue with external sources when serving the browser bundle from a CDN: with DevTools open, this would lead to dozens of HTTP requests to retrieve all the sources. Which is why I tested external sources for the ESM/CJS builds, and inline sources for the browser build: and here, the gains are negligible (-1.2%). (And when we only include sourcemaps for the browser build, then external sources are worse than inline sources.)

So the useful strategies would be:

  1. No change: keep sourcemaps with inline sources.
  2. Medium change:
    • Config: no sourcemaps for ESM/CJS builds, keep a sourcemap with inline sources for the browser build.
    • Outcome: 38% reduction in package size.
  3. Biggest change:
    • Config: no sourcemaps at all.
    • Outcome: 67% reduction in package size.

My vote would be for the first or second option. I wouldn't go for the third option (no sourcemaps at all), because when loading the browser bundle in a web page, it's rather useful to have a source map loaded by DevTools (I tried it, the DX is pretty nice!).

It's a bit unfortunate that the browser build is bloating this package, but unless the goal is to remove the browser build completely and only point users to https://esm.sh/es-toolkit@%5E1/compat (which can be used in the browser with <script type="module">), I think I would keep it and keep its sourcemap.

JSDoc in outputs

If the decision is to remove sourcemaps for the ESM/CJS builds, it might be useful to try to restore JSDoc comments. Currently we keep comments in declaration files, but remove them in .js and .mjs outputs.

Here are the results with different removeComments settings for the ESM/CJS build and their declaration files, for the 3 sourcemaps strategies outlined above:

ESM/CJS sourcemaps Browser sourcemap JSDoc in JS output JSDoc in types .tgz size .tar size
📄 Inline sources 📄 Inline sources ✅ Yes ✅ Yes 192,613 (+7%) 1,674,240 (+11%)
📄 Inline sources 📄 Inline sources 🚫 No ✅ Yes 179,599 1,512,448
📄 Inline sources 📄 Inline sources 🚫 No 🚫 No 169,169 (-6%) 1,338,368 (-12%)
🚫 No sourcemap 📄 Inline sources ✅ Yes ✅ Yes 124,507 (-31%) 1,106,944 (-27%)
🚫 No sourcemap 📄 Inline sources 🚫 No ✅ Yes 110,336 (-39%) 955,392 (-37%)
🚫 No sourcemap 📄 Inline sources 🚫 No 🚫 No 88,310 (-51%) 781,312 (-48%)
🚫 No sourcemap 🚫 No sourcemap ✅ Yes ✅ Yes 82,255 (-54%) 915,968 (-39%)
🚫 No sourcemap 🚫 No sourcemap 🚫 No ✅ Yes 68,299 (-62%) 764,416 (-49%)
🚫 No sourcemap 🚫 No sourcemap 🚫 No 🚫 No 46,092 (-74%) 590,336 (-61%)

If we focus on the middle rows (no sourcemaps for ESM/CJS build, but do publish a sourcemap for the browser build), it might be interesting to keep JSDoc comments in both the JS output (.js and .mjs) and in types (.d.ts and .d.mts):

Possible tweak: derive browser build from ESM build

Finally, if choosing to have only one source map for the browser build, and if we want to have a similar debugging experience in different contexts (browser build from CDN, bundlers and JS runtimes), it's also possible to generate the dist/browser.global.js build from the ESM build, by changing the Rollup input to dist/compat/index.mjs (instead of src/compat/index.ts).

As a result, the TypeScript sources would not end up in the published package at all in their original form. The package also shrinks by a further -6% (because dist/browser.global.js.map gets a bit smaller and compresses better since it duplicates content from dist).

And the debugging experience in DevTools would look like this:

Screenshot 2024-07-25 at 16 22 53

instead of:

Screenshot 2024-07-25 at 11 13 38