Ability to use Map as a stateless component

[Pessimistress]

Motivation

Most modern web applications use a reactive/MVC UI framework, such as React, Angular and Vue. When building a complex application that contains a map component, it is very rare that the map library itself provides all the necessary functionalities. Some typical use cases include:

• Synchronize multiple Map instances, side-by-side, minimap, etc.
• Synchronize the Map camera with an external overlay, e.g. deck.gl or custom UI that moves with the map
• Match the Map with non-geospatial UI, e.g. a list of visible points of interests.

These scenarios all concern two-way state sharing between Map instances and other UI components. In the reactive programming paradigm, an exemplary implementation manages the camera state at the app's top level with a state store. Any UI component can request a change to that state. When the state does change, a rerender is triggered that flows down to the map and other components.

The design that maplibre-gl inherited from mapbox-gl does not make this easy. When the map receives user input, it immediately modifies the camera (transform) and rerenders. A move event is fired after the fact. Say the application updates its state based on a move event listener. When the other React components get rendered, they are always one animation frame behind the map.

I have maintained react-map-gl for over five years. There had been different attempts to address this issue, all concerning a) blocking the map from updating its own camera b) forcing the map to redraw when React rerenders. Prior to v6, the wrapper sets interactive: false and implements its own input handlers. In v7, the wrapper swaps out map.transform with a "shadow transform" that matches the React props just before repaint. Both approaches have painful shortcomings including performance and a creeping amount of hacks.

mapbox-gl issue regarding the stateful nature of input handling
mapbox-gl issue regarding the state dependency of transform

Proposal

Whenever map.transform is about to be modified (any operation that leads to a move event), fire a premove event with the proposed changes: zoom, pitch, bearing, center. The event listeners are allowed to mutate the proposed camera parameters. map.transform is then updated to the "approved" values. In pseudo code:

/// Current
const tr = map.transform;
tr.bearing = newBearing;
tr.pitch = newPitch;
tr.zoom = newZoom;
tr.setLocationAtPoint(around, aroundPoint);

fireMoveEvents();

/// Proposed
const tr = map.transform;
const tr2 = tr.clone(); // this will resolve any incoming change and apply constraints

tr2.bearing = newBearing;
tr2.pitch = newPitch;
tr2.zoom = newZoom;
tr2.setLocationAtPoint(around, aroundPoint);

const proposedChanges = {
  center: tr2.center,
  bearing: tr2.bearing,
  pitch: tr2.pitch,
  zoom: tr2.zoom,
};
map.fire(new Event('premove'), {proposedChanges});

tr.bearing = proposedChanges.bearing;
tr.pitch = proposedChanges.pitch;
tr.zoom = proposedChanges.zoom;
tr.center = proposedChanges.center;

fireMoveEvents();

The current usage pattern and code path will stay the same if the event is not handled.

In an application (or wrapper library) where the Map instance is used as a stateless component, premove is used to block the camera from updating immediately. Rather, the transform will be modified when the state change propogates.

Some of the handler classes will need to be updated. They currently produce panDelta, pitchDelta, zoomDelta etc. for each input event and assume that map.transform holds the accumulated change. Once the transform state and user intention are decoupled, the handlers will need to track the accumulated changes themselves, so that interaction is still responsive and not dependent on when rerender happens.

Here is a very rough proof-of-concept, only enough changes for pointer inputs to work correctly:
https://github.com/maplibre/maplibre-gl-js/compare/x/premove-poc

I'm open to alternative ideas.

Concerns

• It's breaking this library's API convention to have event handlers pass data back.
• Changes to input handlers have rippling effects throughout the code base.
• Is it possible to write test cases that block future commits that assume immediate state change?

[HarelM]

Thanks for opening this discussion!
I haven't looked deeply into the mapbox issues yet, but I was wondering why this should be an event and not a "registered" method:

map.registerPreMove(() => do something here...)

and then use it inside the code above, a bit similar to what we did in addProtocol.

Would that improve some of the concerns?

Not sure about the deltas, I'm not familiar with those unfortunately...

[wipfli]

I think if the usability of MapLibre GL JS in reactive frameworks would get much better, we could maybe justify a breaking change in the API...

[birkskyum]

Thank you for raising this issue

[birkskyum]

We have many handlers - I marked the ones currently adapted in the x/premove branch you mention, to make sure we don't miss something.

• BlockableMapEventHandler in 'src/ui/handler/map_event'
• BoxZoomHandler in 'src/ui/handler/box_zoom'
• ClickZoomHandler in 'src/ui/handler/click_zoom'
• DoubleClickZoomHandler in 'src/ui/handler/shim/dblclick_zoom'
• DragPanHandler in 'src/ui/handler/shim/drag_pan'
• DragRotateHandler in 'src/ui/handler/shim/drag_rotate'
• KeyboardHandler in 'src/ui/handler/keyboard'
• MapEventHandler in 'src/ui/handler/map_event'
• MouseRotateHandler in 'src/ui/handler/mouse'
• MousePanHandler in 'src/ui/handler/mouse'
• MousePitchHandler in 'src/ui/handler/mouse'
• ScrollZoomHandler in 'src/ui/handler/scroll_zoom'
• TapDragZoomHandler in 'src/ui/handler/tap_drag_zoom'
• TapZoomHandler in 'src/ui/handler/tap_zoom'
• TouchPanHandler in 'src/ui/handler/touch_pan'
• TouchPitchHandler in 'src/ui/handler/touch_zoom_rotate'
• TouchRotateHandler in 'src/ui/handler/touch_zoom_rotate'
• TouchZoomHandler in 'src/ui/handler/touch_zoom_rotate'
• TouchZoomRotateHandler in 'src/ui/handler/shim/touch_zoom_rotate'

[HarelM]

I honestly don't like the fact that you fire an event to get a value for something.
It's OK if you fire an event and listen to other event in order to be notified about something, but reading code that looks like fire('myEvent', (data) => use data) is prone to errors and not a good pattern, IMHO.
If there's a need to introduce a message-bus in order to decouple events from who uses them it might be an option (although feels like an overkill for this small library).

[birkskyum]

Okay, so the premove should be made a middleware where we inject a function instead of fire an event.

[HarelM]

yup, this is my suggestion above :-)

[Pessimistress]

@birkskyum Sorry about the mixed up terms. When I said "breaking this library's API convention to have event handlers pass data back" I was referring to the premove event listener. I share the concern with everyone on this thread about that API.

Another option is to follow the convention of transformRequest and have something like transformCameraUpdate. Given the target use case it does not have to be dynamically modified once the Map is constructed.

[HarelM]

transformCameraUpdate sounds like a good direction :-)

[birkskyum]

We already have the synchronous redraw() that will let react drive the render cycle. #300

maplibre-gl-js/src/ui/map.ts

Lines 2738 to 2754 in b7beec3

	/**
	* Force a synchronous redraw of the map.
	* @example
	* map.redraw();
	* @returns {Map} `this`
	*/
	redraw(): Map {
	if (this.style) {
	// cancel the scheduled update
	if (this._frame) {
	this._frame.cancel();
	this._frame = null;
	}
	this._render(0);
	}
	return this;
	}

That is a good start. Maybe we should look into a mode where the asynchronous life cycle MapLibre has is completely disabled or replaced with the redraw() so we don't have to request and then stop the frames all the time.

[birkskyum]

This seems to be similar to Deck.onBeforeRender.

[birkskyum]

@Pessimistress , did you have a chance to look at this? I remember there was some talk in January about adding this to v3, which is getting close now, so it's a bit of a last call.

[Pessimistress]

I'll make another pass on the proposal this week.