Rename CSS classes from mapboxgl-* to maplibre-*

[nyurik]

Question to @maplibre/core -- should all CSS classes be renamed to maplibregl-*, or do we want to keep the original mapboxgl-* to be compatible with other code? Would this be a stylistic or a legal requirement? My understanding is that this would be part of the API, and thus could stay as is. Also I suspect this change would be a relatively painful for the endusers.

[lseelenbinder]

My understanding is that this would be part of the API, and thus could stay as is.

I believe this is the case, so I'm happy with it for now.

Another option is to have both (since CSS classes are cheap), and deprecate the mapboxgl-* eventually.

[rbrundritt]

The only potential issue I can think of is that this may be considered a breaking change for existing apps as others may have modified or overridden some styles. That said, since this will initially be an NPM package/source code and not a hosted service that snaps to the latest version in production, this is lower risk as developers would have to accept the update and should test before pushing to production.

I like the idea of changing the name. I wouldn't want both names as that would mean larger file sizes.

Something else to consider for alignment, perhaps we should expose a maplibre namespace as an alternative to the mapbox namespace. The basic idea window.maplibre = window.mapbox;. Then a dev could use either namespace, and we could document the maplibre namespace but have the mapbox namespace sitting there in the background for backwards compatibility.

[rbrundritt]

Related to #27

[snickell]

We should totally do this, and should totally NOT do this until we have a fully stable fully compatible release..... our current um, velocity, makes me wonder about the wisdom of doing anything other than the barest maintenance..... if we bite more, we have to chew more, and it looks like we're already choking on almost nothing ;-)

[HarelM]

Is this planned soon? Feels like a search and replace mainly...?
The first version was released, so this can be done right?
I think the sooner we do the breaking changes the better it will be for users who do the migration early on so after doing this migration they won't need to work hard in the future...?

[aprilmay]

Is there any reason why the main container has still the mapboxgl-canvas-container class (see in src/ui/map.js)? Shouldn't it be maplibregl-canvas-container, for consistency?

[HarelM]

I might have missed it my mistake, feel free to submit a PR to fix it.

[p-j]

Just so you now, the changes that have been merged are breaking the integration of maplibre with visgl/react-map-gl

For reference this is the documentation for using maplibre with react-map-gl: using with a mapbox-gl fork

As noted here and in the related PR, this likely should have been major bump since it's a breaking change for the larger ecosystem.

IMO, going with a double notation (.maplibregl-*, .mapboxgl-*) would have been a better way to deprecate the use of mapbox named classes.

Would you consider a PR to add back the .mapboxgl- names on top of the .maplibregl- ones ?

[HarelM]

Let's continue forward guys.
The version was released, the mistake has been made.
I don't see a reason to increase the package size just because the version number is incorrect.
I also don't think this is just another fork, we are ultimately going to diverge from mapbox-gl so I think the sooner the better.
If a fork with this can be helpful you are free to create a fork that is usable for the project above.
I have forked ngx-mapbox-gl to ngx-maplibre-gl under this company for this reason. I'm currently maintain it with the help of the community. I tend to think this would be the direction forward unless you would like to do complicated things in order to support both projects.
We encourage migrating project that are wrapping maplibre here. I'm not familiar with your project, but it might fit here... :-)

[p-j]

React Map GL is not my project.

I just happen to be using it in conjunction with maplibre as it was advertised as a mapbox v1 compatible fork.
I followed the conversation at the beginning of maplibre and I thought the intent was to provide an OSS alternative to the new proprietary license of Mapbox v2.

React Map GL is one of the go-to library to do Map Based component in React and was originally started at Uber. It is part of a broader set of libraries to do WebGL visualisation (see https://vis.gl/). I cannot say whether they will be interested in supporting maplibre or not if it drift away from mapbox compatibility too fast/too much.

While I can understand that maplibre doesn't intend to be "just a fork of mapbox" I would encourage you not to ignore the fact that a large ecosystem exists around mapbox v1 and that those tools needs time to adapt.

Adding back the .mapboxgl-* in a v1.15.2 to fix the breaking change introduced in the v1.15.0 and remove them in a v2 that would be the starting point of an incompatible version with mapbox v1 seems the best course of action from a open source ecosystem point of view. I would still keep them as the cost of those css classes is pretty low compared to the hurdle of forking and updating every single plugin or library that is built on top of Mapbox, but that's just my point of view.

[astridx]

@p-j Do you also have problems when you include the CSS as discussed here:
visgl/react-map-gl#1498

[p-j]

Yes, the problem is that some React component are created with the mapbox naming scheme as you can see here: https://github.com/visgl/react-map-gl/blob/master/src/components/popup.js#L181-L199

[HarelM]

I must be missing out something, but what's the difference between using version 1.15.x of maplibre and using maplibre 2.x in case it comes out? maplibre 1.13 was created especially for that - for backwards compatibility to mapbox 1.x. all the libraries that are not planning to migrate to maplibre can stay with that version, no problem...
version 1.14 changes the global namespace and version 1.15 changes the css, both are breaking changes...

I'm not the maintainer/creator of ngx-mapbox-gl, see here: Wykks/ngx-mapbox-gl#260.
That didn't stop me from picking this up, forking it and moving this to this company in order to properly support it.

I must be missing out something basic...

[p-j]

You already know that but:
2.x is a major release, it is expected to come with breaking change / migration path.
1.x is a minor release, it is expected to come with new features, without breaking changes.

1.15 introduces breaking changes that makes libraries relying on the previous behavior fail.
Anyone using such library and casually upgrading their maplibre to the latest 1.15 without checking too much will face issues.

There is an extremely easy fix: keep the new maplibregl-* css class and add back the mapboxgl-* css that were removed.
I propose to do it if you don't have the time or the will to do it, but I don't want to spend time on that if there is 0 chance of getting this merge. Hence why I'm asking if that would be considered.

The hypothetical v2 could possibly remove those css classes (or keep them, that would be my preferred course of actions to avoid fragmenting the ecosystem) but at least that would be clear and have a proper migration path.
In case bugs or security fixes are required, they could be released on both v1 and v2 independently.

Currently, keeping a v1.15 with breaking changes and incompatibility with mapbox v1 prevents this fork from ever adding new "compatible" bug fixes or security fix.

[p-j]

And why not fork react-map-gl and adapt it here, same as above: ecosystem fragmentation & time/energy needed to keep in sync with the original repository vs the value it provide to maintain it.

If/when breaking features are introduced to maplibre, then of course it makes sense, but for now, to the best of my knowledge, that isn't the case and so it isn't necessary.

[aprilmay]

As i understand semver, package managers assume that it's safe to upgrade from 1.13 to 1.15 when a dependency is "^1.13".

What's the increase in size if both conventions are supported, for the time being?

Somehow, the good news is that there are complains: people and companies are migrating to maplibre. I think it's important for the maplibre community to send a positive signal and build trust, by allowing this fix.

[astridx]

I think @p-j proposal is a good compromise.

The disadvantage is the size of the package and the extra work. p-j would help with the work and the package size would be reduced again for release 2.0.

The advantage is that everyone can now prepare for version 2.0. Perhaps a solution will be found in the issue visgl/react-map-gl#1513 if it is already known that the CSS classes will be deleted in Maplibre version 2.0.

[HarelM]

Feel free to submit PRs with the relevant rollback (it's not just css) after you test it. You'll need to send another PR afterwards with bringing everything back and change the version again (to 2.0, don't forget updating the changelog).
I think it's more work than you think...
I don't mind clicking merge and creating the relevant git tags to create npm packages accordingly automatically.
I don't expect support for old versions, let's be honest, this is a community project, no one is planning on investing time on it, but ok, whatever...
I think this is a waste of time... bare in mind that I don't want to waste time on review it so please make sure someone else also review this so I can approve.
There's three of you... good luck :-)

[p-j]

Hey @HarelM you seem upset about this and I don't understand why.
I'm not blaming you or anyone for the issue or the work to be done, I'm grateful maplibre exists and for the time and effort you and the other contributors/maintainers are spending on it.
Anyway, I'll take a stab a it this weekend.

[HarelM]

I'm not upset at all, sorry if it came out that way...
I simply would like to spend my time on getting this library forward and pushing forward features I need/want/like (I'm volunteering here as you might image, so I like to invest my after hours time on things that I like :-))
If you feel strongly about it, that's great, invest your time in it. :-)
I would like to spend as less time as possible on this specific issue, as I see little value in this (that might be only me, but that's fine...).
So I would like other people to review it so I'll simply need to "push a button".
It's all good, I just want to make sure we understand who's doing what and what is expected.
If you want to open an issue on it in order to move this to an open issue that's fine too.
Any help maintaining this library is truly appreciated :-) sorry if it wasn't the narrative you guys got...

[mojodna]

As another data point, https://github.com/mapbox/mapbox-gl-geocoder worked fine with 1.14, but now renders incorrectly and doesn't accept any text input after updating to 1.15.x, presumably due to something similar where it's looking for certain mapboxgl-* CSS classes to attach to.

IMO, we really need to re-add the mapboxgl-* CSS classes (and possibly IDs) back (alongside maplibregl-* classes for forward-compatibility) in the 1.15 series so that apps that pin to ^1 don't appear to mysteriously break when using react-map-gl, mapbox-gl-geocoder, or any other dependencies (possibly internal) we don't know about yet.

+1 to @p-j's proposal

[wipfli]

Thanks everyone for your inputs. It is good to have this discussion and I am happy to see that people are using MapLibre GL JS and care about it.

I would suggest to revert the breaking changes and increment the patch version. Once this is done, let's introduce the breaking changes again and go to version 2.

[Joxit]

After the v2 we will continue to maintain the v1? IMO, this will be complicated 😕

[wipfli]

I would only maintain version 2 and leave version 1 behind.

[wipfli]

Or is that a bad idea @Joxit?

[astridx]

I would suggest to revert the breaking changes and increment the patch version. Once this is done, let's introduce the breaking changes again and go to version 2.

What do you mean exactly @wipfli ? Do you want to delete the class maplibre- or use two classes (maplibre- and mapboxgl-) until Maplibre version 2.0?

[wipfli]

The renaming of the CSS objects should have happened together with the other breaking change, which was the renaming of the JavaScript object mapboxgl to maplibregl. We did this when moving from the release candidates of version 1.13 to the first actual release v1.14.0.

[wipfli]

From what I understand, @p-j is interested in working on a pull request which would revert some renaming of CSS objects. The renaming was probably introduced in #185 and #195.

@astridx sorry I was not reading carefully enough what @p-j suggested. Let's just wait for a pull request from them and then discuss there.

[astridx]

I read it as p-j only wants to submit the PR if we agree here that we won't introduce breaking changes until 2.0 and his PR is getting merged. Is that what we are agreeing on?

For me: i would like to have bouth classes unter 2.0. So we have no breaking change and we have our own classes where other libraries can start to work with. So they can prepair for 2.0.

[p-j]

So to clear things up, I'm proposing to make PR(s) that would ultimately gets released as a v1.15.* that would

• add back the mapboxgl-* classname support
• keep the maplibregl-* classname support

Then, for v2, I think a discussion needs to happen to settle whether or not the mapboxgl-* classname should go away, weighting pros & cons:

• ecosystem compatibility
• migration path
• added value to maplibre as an independent project

Maybe this discussion can be started as part of a broader roadmap for v2 to discuss & share what's in it for the project & community at large.

[HarelM]

As @wipfli and I wrote, this is not the only breaking change, making a PR only for the css part is not helpful.

[p-j]

If you are referring to the namespace change in javascript this is addressable with some little configuration (which could be added to the readme/FAQ):

// webpack.config.js
module.export = {
  // ...
  resolve: {
    alias: {
      'mapbox-gl': 'maplibre-gl'
    }
  }
}

or with rollup

// rollup.config.js
import alias from '@rollup/plugin-alias';

module.exports = {
  // ...
  plugins: [
    alias({
      entries: [
        { find: 'mapbox-gl', replacement: 'maplibre-gl' },
      ]
    })
  ]
};

That's why it's been working without issue with the v1.14.1-rc.2

If you are referring to something else, then can you please elaborate?

[Joxit]

I would only maintain version 2 and leave version 1 behind.

I'm OK with that 😉

[HarelM]

But that's a breaking change too even if there's a workaround.
Much like a workaround not to update to version 1.15.
Typescript changes were also made.
Please submit a PR with the motivation and changes.
I think the best approach would be to add the css along the new one, release a patch version and remove them back in version 2.
That won't solve everything, but let's continue forward...

[p-j]

Yes an no.
Renaming the main JS object and having a simple alias functionality is quite common for 'drop-in' replacement.
Changing the CSS class name is effectively changing the API since so many plugins and tools rely on the classname either for styling (inherit, override etc...) or binding event handlers, or extending components.

The first one is "playing nice with the ecosystem" by not colliding with the existing mapbox global object (albeit one can argue that it's unlikely you'll have to deal with both mapboxgl & maplibregl on the same project), the other one is effectively breaking the ecosystem by disconnecting it from a portion of the project's exposed API.

In the case of the namespace, as a developer, I can continue to use all the libraries and tools that have been working with mapbox v1 by adapting my own integration (using the alias workaround).

In the case of the css rename, it breaks many of those tools and plugins and there is nothing I can do while I wait for each projects to decide whether they will support that change or not; given that forking & adapting & maintaining a whole ecosystem is not worth it.

As for whether or not the v1 should be maintained or not, I think that's not for anyone here to decide, but rather to see if issues and bugfixes will be contributed by the community or not.

If members of the community want to focus on v2, while others maintain v1, so be it. I might sound like a broken record to some of you, but the ecosystem of plugins and compatible tools is what makes this all so valuable.
Breaking up with said ecosystem without providing clear advantages over the "legacy" project will fragment the ecosystem further and may prevent maplibre v2 from gaining any sort of traction.

[HarelM]

This conversation is circular. Locking this thread now. I have wrote what I think should be the next steps. Feel free to open a discussion and raise whatever concerns you have.