Allow patch upgrades to D3.js dependency

[timmclean]

We ran into a problem with D3.js that was resolved in a later patch version (v3.5.4). This small change allows patch upgrades to D3.js (3.5.x) while locking the minor version (i.e., it won't upgrade to 3.6.x).

[masayuki0812]

As mentioned in #1177, we experienced compatibility issue when updating d3 with this version syntax. So, I think it's safer to use strict syntax like current one.

[SamMorrowDrums]

Well, are you not compatible with "d3": "3.5.5" ?

I keep having to mark the resolution manually for this, and I have other libs that use D3 that do want the latest. C3 seems to be working perfectly for me with 3.5.5, but I can understand the risk.

Could you maybe add a patch release, when d3 is updated, and you are compatible, if you need to do it manually?

[aendra-rininsland]

Is this more an issue with C3, or more an issue with how D3 is using semver? Nothing in the patch version should be backwards incompatible, so the fact previous patches have broken C3 points to something awry with something they're doing. We really shouldn't have so much trepidation about restricting C3 to D3's minor version number.

@mbostock, any thoughts?

[SamMorrowDrums]

D3 is on 3.5.6 now, is there any move on this? I'd really prefer to trust d3, and then pin the d3 version in C3, only after it breaks again. Even then, we should create an issue for updating d3 ASAP.

This is a cop-out, although an understandable one, from dealing with the fact d3 has in the past broken C3. We need to trust the Semver and raise issues on D3 if they add breaking changes accidentally on a patch version.

When you have to manage large numbers of dependencies on projects, having repos take the attitude of "it broke last time, so I'm not going to update this dep" is counterproductive. It brings C3 out of sync with actively developed plugins.

[aendra-rininsland]

@SamMorrowDrums Yeah, the more I think about it, the more I agree with you. This is the reason why semver exists in the first place. It totally was updating from 3.5.0 to 3.5.3 that broke it last time (and resulted in a dozen new issues in the queue), and we should really have complained vociferously in the D3 queue about either whatever change caused the performance issue or their use of semver.

My vote is to do the following:

1. Change D3 from a standard dependency to a peer dependency.
2. Change the D3 version resolution to ~3.5.0 (I.e., "update the patch version")
3. Set the version of D3 used in the unit testing to whichever one is specified via npm/bower (Currently it's 3.5.0 consumed via CDN).
4. Whinge in the D3 issue queue if they do something that breaks C3.

Thoughts, @masayuki0812?

Edit: Apparently peerDependencies work differently now than when I used them last. Maybe instead, specify D3 as a devDependency in package.json and as a regular dependency in bower.json?

Edit2: As per @thomblake's reasoning, it really should just be a regular ol' dependency.

[aendra-rininsland]

@masayuki0812 Any chance we can get an update on this issue? There've been a few "updating D3" issues and PRs and it'd be really nice to nip them in the bud with this.

@timmclean Am currently thinking the version would be better as ~3.5.4 as that will enforce using the newer version of D3, but am worried people will have stuff built using both C3 and D3 as a dep that will break due to their stuff (and not necessarily C3) being built for 3.5.0. Any thoughts?

[tamzinblake]

Anyone who needs the older version of D3 specifically can depend on the older version of C3 already. I don't think there's a substantial difference between ~3.5.4 and ~3.5.0 going forward from here.

Setting d3 as a devDependency is the wrong thing to do - folks who use a Browserify workflow won't be able to use it in their projects then. And yes, peerDependencies should be avoided at all cost. It's clearly a dependency in this case.

Either way, just getting the version updated would be full of win.

[aendra-rininsland]

@thomblake Ah, very good points.

Right, I think this should probably be merged at this point. Any thoughts, @masayuki0812? 👍

[timmclean]

Both ~3.5.4 and ~3.5.0 seem fine from my end 👍

[alekstorm]

It is possible to have this both ways: 3.5.0 || ~3.5.4 is valid version syntax, and would skip the offending middle versions. Of course, we could also do <=3.5.0 || ~3.5.4, but that seems far too liberal (I doubt d3 1.x is actually supported).

[masayuki0812]

Thanks for working on this. I think ~3.5.0 is ok at this moment because the latest version of d3 looks ok and we can give specific version to d3 if needed.

[timmclean]

Great, thanks!

[aendra-rininsland]

👍 🎉 🎊 Yay!

[tamzinblake]

And there was much rejoicing!

[terinjokes]

@masayuki0812 any chance we can get this published to npm?