🎨: restore component policy replacement and ordering semantics

lively.morphic/components/policy.js

@@ -12,6 +12,14 @@ const TRANSFORM_PROPS = ['extent', 'position', 'rotation', 'scale', 'lineHeight'
 const TEXT_TYPES = ['text', 'label', Text, Label];
 const expressionSerializer = new ExpressionSerializer();
 
+function isTextType (type) {
+  return withSuperclasses(type).some(candidate => {
+    if (TEXT_TYPES.includes(candidate)) return true;
+    const typeName = candidate?.[Symbol.for('__LivelyClassName__')] || candidate?.name;
+    return typeName === 'Text' || typeName === 'Label';
+  });
+}
+
 export function sanitizeSpec (spec) {
   for (let prop in spec) {
     if (spec[prop]?.isDefaultValue) spec[prop] = spec[prop].value;
@@ -108,6 +116,63 @@ function ensureOrder (originalSubmorphs, adjustedSubmorphs = []) {
   return arr.sortBy(adjustedSubmorphs, (spec) => originalSubmorphs?.indexOf(originalSubmorphs?.find(elem => elem.name === spec.name)));
 }
 
+function orderCommandsForForwardAddReferences (commands) {
+  const additionsByName = new Map();
+  const duplicateNames = new Set();
+  for (const command of commands) {
+    const name = command.COMMAND === 'add' && command.props?.name;
+    if (!name) continue;
+    if (additionsByName.has(name)) {
+      additionsByName.delete(name);
+      duplicateNames.add(name);
+    } else if (!duplicateNames.has(name)) {
+      additionsByName.set(name, command);
+    }
+  }
+
+  const commandIndexes = new Map(commands.map((command, index) => [command, index]));
+  const visitState = new Map();
+  const hasDependencyCycle = (command) => {
+    const state = visitState.get(command);
+    if (state === 'visiting') return true;
+    if (state === 'visited') return false;
+    visitState.set(command, 'visiting');
+    const dependency = additionsByName.get(command.before);
+    if (dependency && hasDependencyCycle(dependency)) return true;
+    visitState.set(command, 'visited');
+    return false;
+  };
+  if ([...additionsByName.values()].some(hasDependencyCycle)) return commands;
+
+  const orderedCommands = [];
+  const waitingForAddition = new Map();
+  const deferredCommands = new Set();
+  const releaseWaitingCommands = (name) => {
+    const waitingCommands = waitingForAddition.get(name) || [];
+    waitingForAddition.delete(name);
+    for (const command of waitingCommands) {
+      deferredCommands.delete(command);
+      orderedCommands.push(command);
+      releaseWaitingCommands(command.props?.name);
+    }
+  };
+
+  for (const command of commands) {
+    const dependency = command.COMMAND === 'add' && additionsByName.get(command.before);
+    if (dependency && commandIndexes.get(dependency) > commandIndexes.get(command)) {
+      const waitingCommands = waitingForAddition.get(command.before) || [];
+      waitingCommands.push(command);
+      waitingForAddition.set(command.before, waitingCommands);
+      deferredCommands.add(command);
+      continue;
+    }
+    orderedCommands.push(command);
+    if (command.COMMAND === 'add') releaseWaitingCommands(command.props?.name);
+  }
+
+  return deferredCommands.size ? commands : orderedCommands;
+}
+
 /**
  * Merges two different specs.
  */
@@ -137,6 +202,7 @@ function mergeInHierarchy (
   }
   // finally we apply the commands
   if (!executeCommands) return;
+  commands = orderCommandsForForwardAddReferences(commands);
   for (let cmd of commands) {
     if (cmd.COMMAND === 'remove' && root.submorphs) {
       const morphToRemove = root.submorphs.find(m => m.name === cmd.target);
@@ -154,14 +220,15 @@ function mergeInHierarchy (
         if (typeof specOrPolicyToAdd.position === 'undefined') delete specOrPolicyToAdd.position;
         if (typeof specOrPolicyToAdd.rotation === 'undefined') delete specOrPolicyToAdd.rotation;
         specOrPolicyToAdd.name = morphToReplace.name;
-        addFn(root, cmd.props, morphToReplace);
+        addFn(root, cmd.props, morphToReplace, true);
         removeFn(root, morphToReplace);
       }
     }
 
     if (cmd.COMMAND === 'add') {
       if (!root.submorphs) root.submorphs = [];
-      const beforeMorph = cmd.before && root.submorphs.find(m => m.name === cmd.before);
+      const beforeMorph = cmd.before && root.submorphs.find(m =>
+        m.name === cmd.before || (m.COMMAND === 'add' && m.props?.name === cmd.before));
       addFn(root, cmd.props, beforeMorph);
     }
   }
@@ -829,10 +896,13 @@ export class StylePolicy {
         });
       };
 
-      const handleAdd = (parent, toBeAdded, before) => {
+      const handleAdd = (parent, toBeAdded, before, isReplacement = false) => {
         // insert add command directly into the baseSpec
         const index = before ? parent.submorphs.indexOf(before) : parent.submorphs.length;
-        if (!toBeAdded.isPolicyApplicator) {
+        if (isReplacement && !toBeAdded.isPolicy) {
+          // A fresh replacement must not synthesize styles from the old same-named node.
+          toBeAdded = new klass(toBeAdded);
+        } else if (!toBeAdded.isPolicyApplicator) {
           toBeAdded = this.generateBaseSpecFromLocal(toBeAdded);
         }
         if (!toBeAdded.name) toBeAdded.name = this.generateUniqueNameFor(toBeAdded);
@@ -849,7 +919,7 @@ export class StylePolicy {
           parentSpec.viewModel = obj.deepMerge(parentViewModel, localSpec.viewModel);
         }
 
-        const isTextSpec = arr.intersect(TEXT_TYPES, withSuperclasses(parentSpec.type)).length > 0;
+        const isTextSpec = isTextType(parentSpec.type);
 
         // handle text and attribute merging
         if (localSpec.textAndAttributes && parentSpec.textAndAttributes &&
@@ -1244,7 +1314,8 @@ export class StylePolicy {
       parentSpec,
       nextLevelSpec);
 
-    const isTextSpec = arr.intersect(TEXT_TYPES, withSuperclasses(subSpec.type)).length > 0;
+    const specType = subSpec.type || nextLevelSpec.type || parentSpec.type;
+    const isTextSpec = isTextType(specType);
 
     if (isTextSpec) {
       subSpec = handleTextProps({ ...subSpec });
@@ -1268,10 +1339,7 @@ export class StylePolicy {
    */
   getSubSpecFor (submorphName, includeWithoutCalls = false, unwrapAddCalls = true) {
     if (!submorphName) return this.spec; // assume we ask for root
-    let embeddedRes;
-
-    let matchingNode = this.lookForMatchingSpec(submorphName, this.spec, includeWithoutCalls);
-    if (embeddedRes) matchingNode = embeddedRes;
+    const matchingNode = this.lookForMatchingSpec(submorphName, this.spec, includeWithoutCalls);
     return matchingNode ? (unwrapAddCalls && matchingNode.props) || matchingNode : null;
   }
 
@@ -1435,19 +1503,23 @@ export class StylePolicy {
    */
   lookForMatchingSpec (specName, spec = this.spec, includeWithoutCall = false) {
     let embeddedRes;
-    return tree.find(spec, node => {
+    const matchingNode = tree.find(spec, node => {
       // handle added morphs
       if (includeWithoutCall && node.COMMAND === 'remove') return node.target === specName;
       if (node.COMMAND === 'add') return node.props.name === specName;
       // handle text and attributes (embedded morphs)
-      if (node.textAndAttributes?.find(textOrAttr => {
-        if (embeddedRes) return;
-        if (textOrAttr?.__isSpec__) embeddedRes = this.lookForMatchingSpec(specName, textOrAttr);
-        if (textOrAttr?.isPolicy && textOrAttr?.name === specName) embeddedRes = textOrAttr;
-      })) return !!embeddedRes;
+      for (const textOrAttr of node.textAndAttributes || []) {
+        if (textOrAttr?.__isSpec__) {
+          embeddedRes = this.lookForMatchingSpec(specName, textOrAttr);
+        } else if (textOrAttr?.isPolicy && textOrAttr.name === specName) {
+          embeddedRes = textOrAttr;
+        }
+        if (embeddedRes) return true;
+      }
       // handle "normal" case
       return node.name === specName;
-    }, node => node.submorphs || node.props?.submorphs) || null;
+    }, node => node.submorphs || node.props?.submorphs);
+    return embeddedRes || matchingNode || null;
   }
 }
 
@@ -1689,6 +1761,11 @@ export class PolicyApplicator extends StylePolicy {
     if (this.isStaleComponentContext) return;
     if (change.value === this) return;
 
+    if (changedMorph !== this.targetMorph) {
+      const ownerMaster = arr.findAndGet(changedMorph.ownerChain(), morph => morph.master);
+      if (ownerMaster && ownerMaster !== this) return;
+    }
+
     let subSpec = this.ensureSubSpecFor(changedMorph);
     if (!subSpec) return;
     if (subSpec?.isPolicyApplicator) {
@@ -1773,6 +1850,11 @@ export class PolicyApplicator extends StylePolicy {
     if (removedMorphSpec) {
       arr.remove(ownerSpec.submorphs, removedMorphSpec);
     }
+    for (const subSpec of ownerSpec.submorphs || []) {
+      if (subSpec.COMMAND === 'add' && subSpec.before === removedMorph.name) {
+        subSpec.before = null;
+      }
+    }
     if (insertRemoveIfNeeded && !removedMorph.__wasAddedToDerived__) {
       // insert the without call, but only for non propagation changes
       if (!ownerSpec.submorphs) ownerSpec.submorphs = [];

lively.morphic/tests/component-policy-regression-test.cp.js

@@ -0,0 +1,121 @@
+/* global describe, it, System */
+import { expect } from 'mocha-es6';
+import { Color } from 'lively.graphics';
+import { ComponentDescriptor, Morph, Text } from 'lively.morphic';
+import { withSuperclasses } from 'lively.classes/util.js';
+import { component, part, replace } from '../components/core.js';
+import { PolicyApplicator } from '../components/policy.js';
+
+const moduleId = import.meta.url.replace(System.baseURL, '');
+
+describe('component policy regressions', () => {
+  it('E finds the policy of a part embedded in text', () => {
+    const Inner = ComponentDescriptor.for(() => component({
+      name: 'inner', fill: Color.orange
+    }), { exportedName: 'EmbeddedInner', moduleId });
+    const Holder = ComponentDescriptor.for(() => component({
+      type: Text,
+      name: 'holder',
+      textAndAttributes: ['x: ', null, part(Inner, { name: 'embedded' }), null]
+    }), { exportedName: 'EmbeddedHolder', moduleId });
+
+    const embedded = Holder.stylePolicy.getSubSpecFor('embedded');
+    expect(embedded, 'E: embedded lookup must return its policy').to.be.instanceof(PolicyApplicator);
+    expect(embedded.name, 'E: lookup must not return the enclosing Text').equals('embedded');
+    expect(embedded.parent, 'E: embedded part must retain its component').equals(Inner.stylePolicy);
+  });
+
+  it('F leaves descendants controlled by a different master alone', () => {
+    const A = ComponentDescriptor.for(() => component({
+      name: 'a',
+      submorphs: [{ name: 'scope', submorphs: [{ name: 'leaf', fill: Color.red }] }]
+    }), { exportedName: 'OuterMaster', moduleId });
+    const B = ComponentDescriptor.for(() => component({
+      name: 'b', submorphs: [{ name: 'leaf', fill: Color.green }]
+    }), { exportedName: 'InnerMaster', moduleId });
+    const instance = part(A);
+    const scope = instance.get('scope');
+    scope.master = B;
+    scope.master.applyIfNeeded(true);
+    const leaf = scope.get('leaf');
+    expect(leaf.fill, 'F: the inner master must be active before the edit').equals(Color.green);
+    expect(instance.master.getSubSpecFor('leaf'), 'F: outer instance starts without a leaf override').deep.equals({ name: 'leaf' });
+
+    leaf.fill = Color.blue;
+
+    expect(instance.master.getSubSpecFor('leaf'), 'F: another master must not gain a leaf override').deep.equals({ name: 'leaf' });
+    instance.master.applyIfNeeded(true);
+    expect(leaf.fill, 'F: reapplying the outer master must preserve the inner override').equals(Color.blue);
+  });
+
+  it('F still records overrides through owners without their own master', () => {
+    const Base = ComponentDescriptor.for(() => component({
+      name: 'base',
+      submorphs: [{ name: 'branch', submorphs: [{ name: 'leaf', fill: Color.red }] }]
+    }), { exportedName: 'UnmasteredBranch', moduleId });
+    const instance = part(Base);
+    const leaf = instance.get('leaf');
+
+    leaf.fill = Color.blue;
+
+    expect(instance.master.getSubSpecFor('leaf').fill, 'F: the nearest master must retain a local override').equals(Symbol.for('lively.skip-property'));
+    instance.master.applyIfNeeded(true);
+    expect(leaf.fill, 'F: reapplying the owning master must preserve its local override').equals(Color.blue);
+  });
+
+  it('G clears a runtime add command referring to a removed sibling', () => {
+    const Base = ComponentDescriptor.for(() => component({
+      name: 'base', submorphs: [{ name: 'y' }]
+    }), { exportedName: 'RemovalBase', moduleId });
+    const instance = part(Base);
+    const y = instance.get('y');
+    const x = new Morph({ name: 'x' });
+    instance.addMorph(x, y);
+    // Live reconciliation records the command after the component is constructed.
+    const addition = instance.master.ensureSubSpecFor(x, true);
+    addition.before = y.name;
+    expect(addition.COMMAND, 'G: setup must retain a runtime add command').equals('add');
+    expect(instance.master.getSubSpecFor('x', false, false), 'G: the command must belong to the active policy').equals(addition);
+    expect(addition.before, 'G: setup must retain the sibling reference').equals('y');
+
+    y.remove();
+    instance.master.removeSpecInResponseTo({ target: instance, args: [y] });
+
+    expect(addition.before, 'G: removal must clear the dangling before reference').equals(null);
+    expect(instance.master.getSubSpecFor('y', true), 'G: the inherited sibling must be marked removed').deep.equals({ COMMAND: 'remove', target: 'y' });
+    expect(instance.master.asBuildSpec().submorphs.map(spec => spec.name), 'G: rebuilding must retain the added morph').deep.equals(['x']);
+  });
+
+  it('A normalizes text for a subclass of a distinct Text class', async () => {
+    const { Text: ReloadedText } = await System.import('lively.morphic/text/morph.js?fx3-distinct-text');
+    class TextSubclass extends ReloadedText {}
+    expect(ReloadedText, 'A: the reloaded Text identity must differ').not.equals(Text);
+    expect(withSuperclasses(TextSubclass).includes(Text), 'A: setup must exercise the class-name fallback').equals(false);
+    const Base = ComponentDescriptor.for(() => component({
+      type: TextSubclass, name: 'distinct text', textString: 'inherited'
+    }), { exportedName: 'DistinctTextBase', moduleId });
+    const Derived = ComponentDescriptor.for(() => component(Base, {
+      textAndAttributes: ['overridden', { fontWeight: 'bold' }]
+    }), { exportedName: 'DistinctTextDerived', moduleId });
+    const spec = Derived.stylePolicy.synthesizeSubSpec();
+
+    expect(spec.textString, 'A: inherited textString must not survive text normalization').equals(undefined);
+    expect(part(Derived).textAndAttributes, 'A: the distinct Text subclass must receive the override').deep.equals(['overridden', { fontWeight: 'bold' }]);
+  });
+
+  it('D replaces a Text with a fresh plain morph without an add command', () => {
+    const Base = ComponentDescriptor.for(() => component({
+      name: 'base', submorphs: [{ type: Text, name: 'old', textString: 'old text' }]
+    }), { exportedName: 'PlainReplacementBase', moduleId });
+    const Derived = ComponentDescriptor.for(() => component(Base, {
+      submorphs: [replace('old', { name: 'replacement', fill: Color.orange })]
+    }), { exportedName: 'PlainReplacementDerived', moduleId });
+    const instance = part(Derived);
+    const replacement = instance.get('old');
+
+    expect(replacement.isText, 'D: a plain replacement must not inherit the old Text type').not.equals(true);
+    expect(replacement.textString, 'D: a plain replacement must not inherit the old text').equals(undefined);
+    expect(replacement.fill, 'D: the replacement must retain its own style').equals(Color.orange);
+    expect(instance.submorphs.map(m => m.name), 'D: the replacement must keep the target name and slot').deep.equals(['old']);
+  });
+});