[geoPath]: pointRadius() Accessor pattern

[tomwanzek]

This is more of a question. I noticed that the geo path pointRadius() getter returns:

1. either an accessor function, or
2. a constant numeric value (i.p. the 4.5 default)

If I recall correctly, in a fair share of the other D3 modules, similar accessors will always return an accessor function, even if it just wraps a constant return value.

Understanding that changing this would be breaking, I simply wondered, if there is a rationale for it, which is specific to the d3-geo applications?

[mbostock]

See d3/d3-shape#23, and earlier, d3/d3#1145.

In the past, I tried to preserve the type as it was set, mostly to facilitate debugging. So, if the value was set as a constant, it would be returned as a constant. In 4.0, I think I generally standardized on always promoting constants to functions. The path.pointRadius is treated a little specially because it can be optimized to set contextStream.pointRadius once if the radius is a constant, rather than always boxing it with a function. That avoids initializing the circle string (see path/string.js) for rendering each feature, but a better optimization would be to initialize the circle string lazily.

I’d probably be okay with changing the behavior without a major release, as the documentation does say that it returns the “current radius accessor” and not the “current radius”, and so this should probably be considered a bug. But yes, there’s a chance this could break something, and it doesn’t seem hugely important either way. 😁

[tomwanzek]

Thanks, like I said, I thought you might have had a reason. So I leave it up to you to find the best fit for this particular accessor (rather than just blindly following a pattern.) I have just pushed updated d3-geo definitions (1.3.1) to DefinitelyTyped. So once they are merged/published everything is in synch with D3 proper.

The definitions in this latest version return a union-type of either number or "accessor function signature". If the module changes we can change that as well.

I am considering a follow-up commit/PR anyway, because I might want to break-out the composite projection interface from the more complete projection interface (i.e. the unsupported methods).