[Bug] 7.0->7.1 upgrade breaks when using MapLibre

[graue]

Description

npm is unable to upgrade react-map-gl from 7.0.21 to 7.1.2 while using MapLibre:

npm ERR! code ERESOLVE
npm ERR! ERESOLVE could not resolve
npm ERR! 
npm ERR! While resolving: [email protected]
npm ERR! Found: [email protected]
npm ERR! node_modules/mapbox-gl
npm ERR!   mapbox-gl@"npm:empty-npm-package@^1.0.0" from the root project
npm ERR!   peer mapbox-gl@">=0.32.1 <2.0.0" from @mapbox/[email protected]
npm ERR!   node_modules/@mapbox/mapbox-gl-supported
npm ERR!     @mapbox/mapbox-gl-supported@"^1.5.0" from [email protected]
npm ERR!     node_modules/maplibre-gl
npm ERR!       maplibre-gl@"^1.15.2" from the root project
npm ERR!       1 more (react-map-gl)
npm ERR! 
npm ERR! Could not resolve dependency:
npm ERR! peerOptional mapbox-gl@">=1.13.0" from [email protected]
npm ERR! node_modules/react-map-gl
npm ERR!   react-map-gl@"^7.0.6" from the root project
npm ERR! 
npm ERR! Conflicting peer dependency: [email protected]
npm ERR! node_modules/mapbox-gl
npm ERR!   peerOptional mapbox-gl@">=1.13.0" from [email protected]
npm ERR!   node_modules/react-map-gl
npm ERR!     react-map-gl@"^7.0.6" from the root project
npm ERR! 
npm ERR! Fix the upstream dependency conflict, or retry
npm ERR! this command with --force or --legacy-peer-deps
npm ERR! to accept an incorrect (and potentially broken) dependency resolution.

This seems to be happening because my dependencies in my package.json has this line, which was the previously recommended way to use react-map-gl with MapLibre:

    "mapbox-gl": "npm:empty-npm-package@^1.0.0",

NPM interprets that as version 1.0.0 of mapbox-gl, while react-map-gl 7.1.2 requires mapbox-gl >1.13.0.

Expected Behavior

To comply with semver, the peer dependency requirement for mapbox-gl should stay "*" (satisfied by 1.0.0) for 7.x versions of react-map-gl, to allow the previously suggested way of using react-map-gl with MapLibre to keep working. You could revert that requirement for now, call that 7.1.3, and then cut a version 8.0 release that changes the dependency, since this is a breaking change.

Steps to Reproduce

Install react-map-gl 7.0.x with MapLibre using the recommended empty-npm-package workaround, then run npm upgrade.

Environment

• Framework version: [email protected]
• Map library: [email protected]
• Browser:
• OS:

Logs

No response

[Pessimistress]

The empty package dependency is defined in your own project. I understand that it was suggested by the v7 documentation as a workaround, however it was never mentioned in react-map-gl's package.json, therefore nothing is broken as far as semantic versioning is concerned. I'm happy to amend the v7.1 upgrade guide to reflect this.

[graue]

Semantic versioning doesn't say anything about things being in package.json or not. It simply says to bump the major version when there's a backwards incompatible API change, which this is. The fix seems easy - release a v7.1.3 that doesn't break the workaround. Why not do that and save people time? Happy to submit a PR.

[graue]

I've written a PR and tested locally that it allows upgrading to work: #2231. Hope you'll consider merging this!

[Pessimistress]

The library does not work with mapbox-gl prior to v1.13.0, hence the restriction. I consider it more important to define the dependencies correctly than supporting an outdated workaround.

[graue]

Is that a problem that has come up in practice -- people trying to use versions of mapbox-gl between 1.0.0 and 1.13.0, and then complaining that it doesn't work?

[Pessimistress]

#2100

[graue]

Okay, but why not release this fix as 8.0, while keeping the 7.x branch from breaking for existing users of the library?

[tordans]

I ran into this as well but removed the empty package wich also resolves the issue. I suggest to update the update guide https://visgl.github.io/react-map-gl/docs/upgrade-guide to remind people to just remove the line if they followed the old workaround before…

[Pessimistress]

why not release this fix as 8.0, while keeping the 7.x branch from breaking for existing users of the library?

The short answer is that I did not think this was a breaking change, but I get your point.
I have opened a PR to make it clear in the upgrade guide that you should remove the placeholder dependency from your package.json.

[Pessimistress]

Published the clarification to upgrade guide.

[graue]

I appreciate your work on ReactMapGL and I realize I may not be able to convince you, but, respectfully, this really doesn't address the problem.

The purpose of semantic versioning is that, for a minor version change, it should not be necessary to check an upgrade guide and work around a breaking change at all. Minor changes should not be breaking.

I would ask once again that you please consider merging #2231 and issuing a new 7.1.x release to fix this bug. You could immediately revert that in a 8.0 release. You'd be saving a lot of people some time and headache.

[Pessimistress]

It is not a breaking API change to require a higher version of some dependency. Packages do so all the time with security updates. Given that the version of mapbox-gl is defined in your own project's package.json, you are responsible for upgrading it.