Norkart / Leaflet-MiniMap

A minimap control plugin for Leaflet
BSD 2-Clause "Simplified" License
395 stars 125 forks source link

update SVGO and other devDependencies #146

Open johnd0e opened 5 years ago

johnd0e commented 5 years ago

Alternative to #145

N.B.:

robpvn commented 5 years ago

Hi, very nice of you to contribute this! I have a couple of issues with this PR, though.

I guess all we really need to do is update svgo and not much else, the way I see it.

johnd0e commented 5 years ago

I don't see the need to update so many dependencies at once when all we want is a better svgo.

Well, I will try to explain my point.

  1. There are 2 kinds of dependencies.

    • dependencies in production, where we should update carefully, because if we change our version requirements we can break other projects that use Leaflet-Minimap (as not every project will comply to that new version of our dependency).
    • devDependencies, where we can use whatever version that we want, and nobody else cares.
  2. We have only 1 dependency of 1st type (and it's incorrectly defined in devDependencies). Technically leaflet 1.4 is fully compatible within minor version changes, but I agree that we can leave it at 1.0.3.

  3. And main question: why we ever need to update any dependency? I have my answer:

    • Node community updates every package day by day making a lot of improvements. Some of them are subtle but still important.
    • Currently dependencies' versions defined in our package.json are extremely outdated. Try npm audit and you see that there are a lot of known vulnerabilities.

Does updating svg2png and rewriting the way it's used get us any improvements?

I believe that we should keep all dependencies in actual state. It's safe, and I do not see any drawback.

I've just updated this PR according to my vision.

It seems you forgot to add pn as a dependency.

It is dependency of svg2png, we have not define it explicitly. Try to build and see.

johnd0e commented 5 years ago

leaflet moved from devDependencies to peerDependancies as demanded by PLUGIN-GUIDE.