-
Notifications
You must be signed in to change notification settings - Fork 124
Description
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/d3-flame-graph@4.1.3/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
distversion, 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
distversion, 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
distfolder for websites, but for npm imports, don't use thedistversion? Like in the top-level index.js, don't export all ofdist/.