fix: reconcile routeselected handlers and persist selected alternative

AGENTS.md

@@ -20,6 +20,10 @@ OSRM Frontend is the browser-based routing interface served at https://map.proje
 
 - All new functionality must have unit test coverage.
 
+### Naming
+
+- **Use descriptive, functional names for variables and functions.** Avoid cute, playful, or whimsical names (e.g. no `makeRoute`, `switchTo`, `handleIt`). Names should describe what the thing is or does, not how the author feels about it.
+
 ### Notes
 
 - Commit generated artifacts (`bundle.js`, `bundle.js.map`, `dist/`) only when necessary.

src/index.js

@@ -43,6 +43,7 @@ var localization = require('./localization');
 var initialLayers = require('./initial_layers');
 var layerUtils = require('./layer_utils');
 var routeZoom = require('./route_zoom');
+var resolveInitialAlternative = require('./route_alternative');
 require('./polyfill');
 
 var parsedOptions = urlState.parse(window.location.search.slice(1));
@@ -721,6 +722,23 @@ lrmControl.on('alternateChosen', function(e) {
 lrmControl.on('routeselected', function(e) {
   var route = e.route || {};
 
+  // On the initial route load, the LRM always selects route[0] first.
+  // If the URL requested a different alternative, switch to it now.
+  // Falls back to route[0] when the requested alternative no longer exists.
+  if (shouldFitRoute) {
+    var switchTo = resolveInitialAlternative(route, e.alternatives, state.options.alternative);
+    if (switchTo) {
+      // Re-fire on lrmControl so the LRM-internal listeners
+      // (_routeSelected, _selectAlt) also process the correct route.
+      lrmControl.fire('routeselected', switchTo);
+      return;
+    }
+  }
+
+  // Track selected alternative in URL state and persist to URL
+  state.options.alternative = route.routesIndex;
+  state.update();
+
   // Route export: build GeoJSON and hand it to the tools control for GPX download
   var routeGeoJSON = {
     type: 'Feature',

src/route_alternative.js

@@ -0,0 +1,32 @@
+'use strict';
+
+/**
+ * Decides whether to switch from the current route to a different alternative
+ * based on a desired index (e.g. read from URL state on initial page load).
+ *
+ * The LRM always selects route[0] first.  If the caller wants a different
+ * alternative, this function returns the re-arranged event payload so the
+ * caller can re-fire `routeselected` with the correct route.
+ *
+ * @param {Object}  route        – the currently selected route (must have .routesIndex)
+ * @param {Array}   alternatives – other route objects (may be empty / undefined)
+ * @param {*}       desiredAlt   – the 0-based index of the desired alternative
+ * @returns {Object|null}        – {route, alternatives} to re-fire, or null when
+ *                                 no switch is needed (desiredAlt is 0, missing,
+ *                                 already matches, or out of bounds)
+ */
+module.exports = function resolveInitialAlternative(route, alternatives, desiredAlt) {
+  var index = Number(desiredAlt);
+  if (!index || index < 0 || index % 1 !== 0) return null;  // 0, NaN, undefined, null, "", negative, non-integer
+  if (route.routesIndex === index) return null; // already on the desired route
+
+  var allRoutes = [route].concat(alternatives || []);
+  if (index >= allRoutes.length) return null;   // desiredAlt no longer exists
+
+  return {
+    route: allRoutes[index],
+    alternatives: allRoutes.filter(function(_, i) {
+      return i !== index;
+    })
+  };
+};

src/state.js

@@ -18,10 +18,6 @@ var State = L.Class.extend({
 
     this.set(default_options);
 
-    this._lrm.on('routeselected', function(e) {
-      this.options.alternative = e.route.routesIndex;
-    }, this);
-
     this._lrm.getPlan().on('waypointschanged', function() {
       this.options.waypoints = this._lrm.getWaypoints(); this.update({ push: true });
     }.bind(this));

test/route_alternative.test.js

@@ -0,0 +1,170 @@
+'use strict';
+
+var resolveInitialAlternative = require('../src/route_alternative');
+
+function makeRoute(routesIndex) {
+  return { name: 'Route ' + routesIndex, coordinates: [], routesIndex: routesIndex };
+}
+
+describe('resolveInitialAlternative', function() {
+
+  describe('returns null (no switch) when', function() {
+
+    test('desiredAlt is 0', function() {
+      var route = makeRoute(0);
+      var result = resolveInitialAlternative(route, [makeRoute(1)], 0);
+      expect(result).toBeNull();
+    });
+
+    test('desiredAlt is undefined', function() {
+      var route = makeRoute(0);
+      var result = resolveInitialAlternative(route, [makeRoute(1)], undefined);
+      expect(result).toBeNull();
+    });
+
+    test('desiredAlt is null', function() {
+      var route = makeRoute(0);
+      var result = resolveInitialAlternative(route, [makeRoute(1)], null);
+      expect(result).toBeNull();
+    });
+
+    test('desiredAlt is NaN (unparseable string)', function() {
+      var route = makeRoute(0);
+      var result = resolveInitialAlternative(route, [makeRoute(1)], 'abc');
+      expect(result).toBeNull();
+    });
+
+    test('desiredAlt is an empty string', function() {
+      var route = makeRoute(0);
+      var result = resolveInitialAlternative(route, [makeRoute(1)], '');
+      expect(result).toBeNull();
+    });
+
+    test('route.routesIndex already matches desiredAlt', function() {
+      var route = makeRoute(1);
+      var result = resolveInitialAlternative(route, [makeRoute(0), makeRoute(2)], 1);
+      expect(result).toBeNull();
+    });
+
+    test('desiredAlt exceeds available routes (out of bounds)', function() {
+      var route = makeRoute(0);
+      // Only 3 routes total (indices 0,1,2) — desiredAlt 5 doesn't exist
+      var result = resolveInitialAlternative(route, [makeRoute(1), makeRoute(2)], 5);
+      expect(result).toBeNull();
+    });
+
+    test('desiredAlt equals length (off by one)', function() {
+      var route = makeRoute(0);
+      // 3 routes, index 3 is past the end
+      var result = resolveInitialAlternative(route, [makeRoute(1), makeRoute(2)], 3);
+      expect(result).toBeNull();
+    });
+
+    test('there are no alternatives and desiredAlt > 0', function() {
+      var route = makeRoute(0);
+      var result = resolveInitialAlternative(route, [], 1);
+      expect(result).toBeNull();
+    });
+
+    test('alternatives is undefined', function() {
+      var route = makeRoute(0);
+      var result = resolveInitialAlternative(route, undefined, 1);
+      expect(result).toBeNull();
+    });
+
+    test('alternatives is null', function() {
+      var route = makeRoute(0);
+      var result = resolveInitialAlternative(route, null, 1);
+      expect(result).toBeNull();
+    });
+
+    test('desiredAlt is negative', function() {
+      var route = makeRoute(0);
+      var result = resolveInitialAlternative(route, [makeRoute(1)], -1);
+      expect(result).toBeNull();
+    });
+
+    test('desiredAlt is a non-integer float (e.g. "1.5")', function() {
+      var route = makeRoute(0);
+      var result = resolveInitialAlternative(route, [makeRoute(1), makeRoute(2)], 1.5);
+      expect(result).toBeNull();
+    });
+
+    test('desiredAlt is a float string ("1.5")', function() {
+      var route = makeRoute(0);
+      var result = resolveInitialAlternative(route, [makeRoute(1), makeRoute(2)], '1.5');
+      expect(result).toBeNull();
+    });
+  });
+
+  describe('returns a switch payload when', function() {
+
+    test('desiredAlt matches a middle alternative', function() {
+      var route0 = makeRoute(0);
+      var route1 = makeRoute(1);
+      var route2 = makeRoute(2);
+      var result = resolveInitialAlternative(route0, [route1, route2], 1);
+
+      expect(result).not.toBeNull();
+      expect(result.route).toBe(route1);
+      expect(result.alternatives).toHaveLength(2);
+      expect(result.alternatives).toContain(route0);
+      expect(result.alternatives).toContain(route2);
+    });
+
+    test('desiredAlt is the last alternative', function() {
+      var route0 = makeRoute(0);
+      var route1 = makeRoute(1);
+      var route2 = makeRoute(2);
+      var result = resolveInitialAlternative(route0, [route1, route2], 2);
+
+      expect(result).not.toBeNull();
+      expect(result.route).toBe(route2);
+      expect(result.alternatives).toEqual([route0, route1]);
+    });
+
+    test('desiredAlt is a string ("1") that coerces to number', function() {
+      var route0 = makeRoute(0);
+      var route1 = makeRoute(1);
+      var route2 = makeRoute(2);
+      // This mimics the URL parser returning strings
+      var result = resolveInitialAlternative(route0, [route1, route2], '1');
+
+      expect(result).not.toBeNull();
+      expect(result.route).toBe(route1);
+      expect(result.alternatives).toContain(route0);
+      expect(result.alternatives).toContain(route2);
+    });
+
+    test('there is exactly one alternative and it is the target', function() {
+      var route0 = makeRoute(0);
+      var route1 = makeRoute(1);
+      var result = resolveInitialAlternative(route0, [route1], 1);
+
+      expect(result).not.toBeNull();
+      expect(result.route).toBe(route1);
+      expect(result.alternatives).toEqual([route0]);
+    });
+  });
+
+  describe('returns referentially correct objects', function() {
+
+    test('the returned route is the exact same object from the input', function() {
+      var route0 = makeRoute(0);
+      var route1 = makeRoute(1);
+      var result = resolveInitialAlternative(route0, [route1], 1);
+
+      expect(result.route).toBe(route1);
+    });
+
+    test('returned alternatives are the exact same objects', function() {
+      var route0 = makeRoute(0);
+      var route1 = makeRoute(1);
+      var route2 = makeRoute(2);
+      var result = resolveInitialAlternative(route0, [route1, route2], 1);
+
+      expect(result.alternatives[0]).toBe(route0);
+      expect(result.alternatives[1]).toBe(route2);
+    });
+  });
+});