Drop the select-zone compatibility fallback: the applier now answers for itself
The cleanup I promised when the fallback went in, done now that both halves are on
main (verified BY CONTENT at ccc2b1bb, not by sha).
onLegendSelect returns an EXPLICIT boolean on all four of its paths — false when the
tapestry layer is not up, true for batch mode, true for the un-isolate, true on
success. No path yields undefined, so the `typeof applied === 'boolean'` capability
check can never fall through and the inference behind it is unreachable. Three lines
become one.
WHY THE CHECK WAS WRITTEN THAT WAY, since the reasoning outlives the code: upg100-plugin
warned that a function with no explicit return yields undefined, which is FALSY — so
adopting their boolean before it landed would have read every success as a failure and
retried it. Testing the CAPABILITY rather than the version meant this worked before
their change, after it, and in any bundle built from either, with no flag day.
AND WHY NOT TO REINTRODUCE THE INFERENCE, which is the part worth keeping: I had read
!selectedKey.value as "it failed". That is a proxy for "did it ISOLATE", not for "did
it work", and the two come apart in exactly two paths — batch mode feeds the selection
union and the un-isolate clears it, and both leave selectedKey unset while having
succeeded. Retaining on that would have re-applied the zone and toggled it back OUT of
the union. upg100-plugin found it prospectively and added the boolean so the applier
could be asked instead of guessed at.
Source only; my-upg-100-list needs the rebuild.
This commit is contained in:
parent
370446390f
commit
aba2dfb710
1 changed files with 13 additions and 16 deletions
|
|
@ -1713,22 +1713,19 @@ function applySelectZone(zoneId) {
|
|||
// map's tapestry layer is not up, which at t=4.9s it was not. So the readiness signal was never
|
||||
// legendNodes or mapRef -- it is THE LAYER, and the old code cleared _pendingZoneId here as though
|
||||
// the work were done. Keeping it means ensureLayers() can drain it the moment the layer exists.
|
||||
// ★ ASK THE APPLIER, DO NOT INFER FROM A SIDE EFFECT -- and the inference was WRONG IN BATCH MODE.
|
||||
// upg100-plugin caught this prospectively: onLegendSelect feeds the selection UNION in batch mode
|
||||
// and returns without ever setting selectedKey, because isolating is not what it should do there.
|
||||
// So `!selectedKey.value` read that correct behaviour as a failure, and retaining on it would have
|
||||
// re-applied the zone and toggled it back OUT of the union. Un-isolating (node == null) is the same
|
||||
// shape: the un-isolate IS the action, and selectedKey is null afterwards BY DESIGN.
|
||||
// They have made onLegendSelect return a boolean for exactly this (fb076c8b on packet/vertical),
|
||||
// where false means one thing only: the map was not ready.
|
||||
// FORWARD-COMPATIBLE ON PURPOSE. That change is not on main yet, and a function with no explicit
|
||||
// return yields undefined, which is FALSY -- so depending on it today would read every success as a
|
||||
// failure and retry it, which is the very trap they warned about. So the boolean is authoritative
|
||||
// WHEN IT IS A BOOLEAN, and until then the old inference stands with batch mode excluded.
|
||||
const ok = (typeof applied === 'boolean')
|
||||
? applied
|
||||
: (batchMode.value ? true : !!selectedKey.value);
|
||||
if (!ok) { _pendingZoneId = zoneId; _markZoneState('applied-but-no-selection', zoneId); return false; }
|
||||
// ★ THE COMPATIBILITY FALLBACK IS GONE, and this is the promised cleanup rather than a change of
|
||||
// behaviour. onLegendSelect now returns an EXPLICIT boolean on all four of its paths — false when
|
||||
// the tapestry layer is not up, true for batch mode (it fed the union), true for the un-isolate
|
||||
// (the un-isolate IS the action) and true on success. So there is no path that yields undefined,
|
||||
// and the `typeof applied === 'boolean'` capability check it guarded can never fall through.
|
||||
//
|
||||
// WHY IT EXISTED AT ALL, kept as the reason not to reintroduce the inference: I used to read
|
||||
// !selectedKey.value as "it failed". That is a proxy for "did it ISOLATE", not for "did it work",
|
||||
// and the two differ in exactly the two paths above — batch mode and the un-isolate both leave
|
||||
// selectedKey unset while having succeeded. Retaining on that would have re-applied the zone and
|
||||
// toggled it back OUT of the selection union. upg100-plugin caught that before it bit anyone, and
|
||||
// added the boolean so the applier could be ASKED instead of inferred from.
|
||||
if (!applied) { _pendingZoneId = zoneId; _markZoneState('applied-but-no-selection', zoneId); return false; }
|
||||
_markZoneState('applied', zoneId);
|
||||
return true;
|
||||
}
|
||||
|
|
|
|||
Loading…
Reference in a new issue