d3-flame-graph icon indicating copy to clipboard operation
d3-flame-graph copied to clipboard

Distribution includes d3 functions, which get duplicated (& maybe version skew) when sites also include d3.js

Open mhansen opened this issue 3 years ago • 2 comments

I think flamegraph/dist bundles a copy of many d3 functions, increasing page weight and risking version skew.

To Reproduce

  • Open https://cdn.jsdelivr.net/npm/[email protected]/dist/d3-flamegraph.js
  • Search for node_modules/d3-
  • note 139 matches, including (a selection of files from top-level modules):
    • node_modules/d3-selection/src/selector.js
    • node_modules/d3-format/src/exponent.js
    • node_modules/d3-hierarchy/src/partition.js
    • node_modules/d3-array/src/ticks.js
    • node_modules/d3-color/src/define.js
    • node_modules/d3-interpolate/src/basis.js
    • node_modules/d3-scale/src/init.js
    • node_modules/d3-ease/src/cubic.js
    • node_modules/d3-dispatch/src/dispatch.js
    • node_modules/d3-timer/src/timer.js
    • node_modules/d3-transition/src/interrupt.js

I think this is true of the minified JS (but to a lesser extent because unused functions will be removed). I'm just using the unminified JS as it clearly demonstrates the files included.

For example, d3-flamegraph.js has this line from d3-transition/src/transition/schedule.js:

  if (schedule.state > CREATED) throw new Error("too late; already scheduled");

d3-flamegraph.min.js also seems to include this:

if(e.state>0)throw new Error("too late; already scheduled");

Expected behavior

  • No d3 functions included, because:
    • The expectation is on the user to include d3.js separately on the webpage, and we want just one version of each d3 function included (to reduce page weight).
    • When used in npm, importing d3-flame-graph imports the dist version, which might (not sure) prevent npm from deduplicating dependencies (say, if your project depends on d3-select and d3-flame-graph, you'll get two versions of d3-select in the output).

I wonder if this might lead to version skew sometimes if d3-flame-graph bundles one version of d3's functions and the site includes another? I suppose this is unavoidable in general; d3-flame-graph calls d3 functions, and expects some set of functions to be there.

I'm not sure the best way to solve this. Here's some brainstorming:

  • Maybe in npm, don't ship a dist version, but rather ship the raw source, depending on d3 libraries? This would prevent duplicating in npm builds. But if we're serving straight from npm to websites, this might break the websites.
  • Maybe we could keep the dist folder for websites, but for npm imports, don't use the dist version? Like in the top-level index.js, don't export all of dist/.

mhansen avatar Dec 18 '21 00:12 mhansen

I should add, I don't know that this is an urgent issue or anything (nothing seems to be really 'breaking' yet!), just thought I'd file it to track in case this was unexpected.

mhansen avatar Dec 18 '21 00:12 mhansen

Thanks for opening the issue @mhansen! This is likely related to a bundling configuration we might have missed during the migration to Webpack, where we need to specify packages that are available already, so they don't get bundled and duplicated. No reports of issues so far (and assuming the same major version is being used, should be fine), but we shouldn't be shipping duplicated code. I'll check the build configs to see if there's anything missing.

spiermar avatar Dec 19 '21 19:12 spiermar