Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[Bug] RTLTextPlugin is eagerly loaded from mapbox.com when using MapLibre and raising privacy concerns #2310

Closed
KiwiKilian opened this issue Nov 6, 2023 · 10 comments
Labels

Comments

@KiwiKilian
Copy link
Contributor

KiwiKilian commented Nov 6, 2023

Description

The RTLTextPlugin prop is set to https://api.mapbox.com/mapbox-gl-js/plugins/mapbox-gl-rtl-text/v0.2.3/mapbox-gl-rtl-text.js per default. This is the same default for both maplibs Mapbox and MapLibre. Furthermore the docs state:

Setting this prop is the equivelant of calling mapboxgl.setRTLTextPlugin with lazy: true.

But this doesn't align with the currently implemented behavior, which loads it eagerly.

To conclude this results in eager loading with both Mapbox and MapLibre of the RTLTextPlugin from the mapbox.com domain. This seems totally counterintuitive when using MapLibre. Especially when viewing from a privacy standpoint. Currently judicial ruling in the EU requires for consent of users before sending requests to US providers from an EU based website.

Therefore I don't expect a library like react-map-gl to fetch from a remote domain I didn't specify myself. Especially when using the MapLibre maplib. Currently it's necessary to set RTLTextPlugin={null}, to prevent from loading the plugin. Which is also not straight-forward as the types do not supporting disabling the RTLTextPlugin.

Expected Behavior

This might be subject to personal opinion again, but I would like to see a breaking change, which removes the RTLTextPlugin default url.

Regardless of this, lazy loading should be enabled or even allow further customization of the loading via the prop like:

RTLTextPlugin={{
  url: "...",
  lazy: true,
}}

It should also be possible to disable RTLTextPlugin, if a default is kept.

Steps to Reproduce

Check the network tab of https://codesandbox.io/s/react-map-gl-maplibre-hfpr6l?file=/src/index.js for resources from mapbox.com.

Environment

  • Framework version: 7.1.6
  • Map library: maplibre-gl 3.5.2
  • Browser: Arc 1.15.1 Chromium 119
  • OS: macOS

Logs

No response

@KiwiKilian KiwiKilian added the bug label Nov 6, 2023
@KiwiKilian KiwiKilian changed the title [Bug] RTLTextPlugin is eagerly loaded from Mapbox when using MapLibre [Bug] RTLTextPlugin is eagerly loaded from mapbox.com when using MapLibre and raising privacy concerns Nov 6, 2023
@Pessimistress
Copy link
Collaborator

Thanks for raising this.

  • the lazy option indeed seems to be a bug, it should be set to true
  • to allow disable the plugin, we could just update the type of RTLTextPlugin to string|false
  • maplibre's RTL example loads the same plugin published by Mapbox. We could point the default value of this prop to unpkg if that makes you feel better (without introducing a breaking change)?

@KiwiKilian
Copy link
Contributor Author

KiwiKilian commented Nov 8, 2023

Thanks for having a look into this! From the privacy point it doesn't matter if it's loaded from mapbox.com or unpkg.com – both are US providers/corporations. Here is a write-up from WordPress about this topic (the most common case is around Google Fonts and GDPR). In the EU it's required to have consent from the users prior to sending requests to US entities. So my preferred move would be the breaking change to also align with the default behavior of the maplibs. I think this is another problem, when switching from plain usage of the maplibs, react-map-gl adds some opinionated behavior to them. But I totally understand this might be not tolerable to create a breaking change for this topic. But maybe it could be kept in the backlog for the next major release?

If you would rather fix it in a minor release, I would also recommend the first to points:

  • Fix it to make it lazy per default
  • Allow to disable RTLTextPlugin with string|false typings

Let me know which way you would like to go and I can create a PR.

@Pessimistress
Copy link
Collaborator

I agree with your concern. If you are up for it, you may submit two PRs, one containing the non-breaking changes, one removing the default. I can publish the first one as a patch and defer the second one to the next major release.

Much appreciated!

@KiwiKilian
Copy link
Contributor Author

Will make those two PRs! Really appreciate the thoughtfulness in the maintenance of this project, joy to work on if other people care too 👍 .

For the breaking change, would you rather stick with defaulting to lazy loading and only providing a string as currently implemented or shall it be configurable like this:

RTLTextPlugin={{
  url: "...",
  lazy: true,
}}

This would give full control to library users. And if it's a breaking change anyway, the structure change is not a big problem.

@exrhizo
Copy link

exrhizo commented Dec 14, 2023

I am also running into RTL being loaded violating content security policy using kepler.gl

Kepler.gl uses: import {Map, MapRef} from 'react-map-gl';

So at most I can get access to forward ref on Map from the kepler side:

  • contextValue.map = createRef(mapbox, mapboxgl);

This createRef doesn't give a handle into mapboxgl globals where I would set false to RTLTextPlugin.

My current workaround is to rewrite the code before bundling..

I don't understand, can this be bundled in the library, rather than dynamically loaded, it's not good for security..

relevant files:

  • kepler.gl/src/components/src/map-container.tsx
  • react-map-gl/src/components/map.tsx
  • react-map-gl/src/mapbox/create-ref.ts

@KiwiKilian
Copy link
Contributor Author

KiwiKilian commented Dec 14, 2023

@Pessimistress could please you give me feedback on how you would like to shape the RTLTextPlugin prop as proposed here #2310 (comment)?

@Pessimistress
Copy link
Collaborator

Fixed in v7.1.7

@seanwessmith
Copy link

seanwessmith commented Aug 30, 2024

@Pessimistress @KiwiKilian
can someone explain how this is fixed? I'm still seeing the URL being loaded when using Map and Marker

import { Map, Marker } from "react-map-gl";

            Promise.resolve(t || r)
              .then((t) => {
                if (!a) return;
                if (!t) throw new Error("Invalid mapLib");
                const r = "Map" in t ? t : t.default;
                if (!r.Map) throw new Error("Invalid mapLib");
                if (
                  ((function (e, t) {
                    for (const n of zn) n in t && (e[n] = t[n]);
                    const {
                      RTLTextPlugin:
                        n = "https://api.mapbox.com/mapbox-gl-js/plugins/mapbox-gl-rtl-text/v0.2.3/mapbox-gl-rtl-text.js",
                    } = t;
                    n &&
                      e.getRTLTextPluginStatus &&
                      "unavailable" === e.getRTLTextPluginStatus() &&
                      e.setRTLTextPlugin(
                        n,
                        (e) => {
                          e && console.error(e);
                        },
                        !0
                      );
                  })(r, e),
                  r.supported && !r.supported(e))
                )
                  throw new Error("Map is not supported by this browser");
                e.reuseMaps && (n = Ln.reuse(e, i.current)),
                  n || (n = new Ln(r.Map, e, i.current)),
                  (c.map = An(n)),
                  (c.mapLib = r),
                  l(n),
                  null == o || o.onMapMount(c.map, e.id);
              })

@Pessimistress
Copy link
Collaborator

@seanwessmith
Copy link

Thank you!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

4 participants