d3 / d3-zoom

Pan and zoom SVG, HTML or Canvas using mouse or touch input.
https://d3js.org/d3-zoom
ISC License
502 stars 143 forks source link

d3.zoomIdentity can change => .translate() does not return a new transform? #174

Closed alexlenail closed 5 years ago

alexlenail commented 5 years ago

Associated fiddle.

The docs read:

To create a transform with a given k, tx, and ty:

var t = d3.zoomIdentity.translate(x, y).scale(k);

I want to store a zoomTransform of a <g>. So I do:

    var svg = d3.select('#container').append('svg').on('wheel', pan)
    var g = svg.append('g');
    var g_transform = d3.zoomIdentity.translate(0, 0).scale(1);

    function pan() {
        g_transform.x -= d3.event.deltaY;
        g.attr('transform', g_transform);
        console.log(d3.zoomIdentity);
    }

It turns out that mutating my g_transform also mutates d3.zoomIdentity, which is surprising, because although I'm warned against mutating zoomTransforms, the docs and code suggest that .translate(0, 0) should return me a new transform object, distinct from d3.zoomIdentity

https://github.com/d3/d3-zoom/blob/6f8b119399f1b68183974ae818b9900514a24df0/src/transform.js#L12-L14

What am I misunderstanding? Should the docs be updated to reflect that .translate does not return a new zoomTransform? Or could this be a bug?

ahmadkarim commented 4 years ago

@alexlenail did you ever figure out why ?

mbostock commented 4 years ago

transform.translate is not guaranteed to return a new transform, and you can see from the source that it does not when the given x and y are zero:

https://github.com/d3/d3-zoom/blob/3c95789cd05cb04b99d568ddf4772d2cb58a8a0e/src/transform.js#L12-L14

In general, the expectation is that you don’t mutate the fields of transform objects and you treat them as immutable.

We don’t currently expose the Transform constructor, but we can add that easily if you want to make a copy of a transform.