Better implementation, improve tests

plone · Feb 20, 2024 · c2d859f · c2d859f
1 parent 81dfe61
commit c2d859f
Show file tree

Hide file tree

Showing 3 changed files with 144 additions and 43 deletions.
diff --git a/packages/registry/src/index.ts b/packages/registry/src/index.ts
@@ -191,16 +191,30 @@ class Config {
       // For all registered components for that slot, inversed, since the last one registered wins
       // TODO: Cover ZCA use case, where if more predicates, more specificity wins if all true.
       // Let's keep it simple here and stick to the registered order.
+      let noPredicateComponent: SlotComponent | undefined;
       for (const slotComponent of data[slotName].toReversed()) {
-        const isPredicateTrueFound = slotComponent.predicates.every(
-          (predicate) => predicate(args),
-        );
+        let isPredicateTrueFound: boolean = false;
+        if (slotComponent.predicates) {
+          isPredicateTrueFound = slotComponent.predicates.every((predicate) =>
+            predicate(args),
+          );
+        } else {
+          // We mark the one with no predicates
+          noPredicateComponent = slotComponent;
+        }
+
         // If all the predicates are truthy
         if (isPredicateTrueFound) {
           slotComponents.push(slotComponent.component);
+          // We "reset" the marker, we already found a candidate
+          noPredicateComponent = undefined;
           break;
         }
       }
+
+      if (noPredicateComponent) {
+        slotComponents.push(noPredicateComponent.component);
+      }
     }
 
     return slotComponents;
@@ -209,55 +223,64 @@ class Config {
   registerSlotComponent(options: {
     slot: string;
     name: string;
-    predicates: ((...args: unknown[]) => boolean)[];
+    predicates?: ((...args: unknown[]) => boolean)[];
     component: React.ComponentType;
   }) {
     const { name, component, predicates, slot } = options;
 
     if (!component) {
       throw new Error('No component provided');
-    } else {
-      let currentSlot = this._data.slots[slot];
-      if (!currentSlot) {
-        this._data.slots[slot] = {
-          slots: [],
-          data: {},
-        };
-        currentSlot = this._data.slots[slot];
-      }
-      if (!currentSlot.data[name]) {
-        currentSlot.data[name] = [];
+    }
+    if (!predicates) {
+      // Test if there's already one registered, we only support one
+      const hasRegisteredNoPredicatesComponent = this._data.slots?.[
+        slot
+      ]?.data?.[name]?.find(({ predicates }) => !predicates);
+      console.log(hasRegisteredNoPredicatesComponent);
+      if (hasRegisteredNoPredicatesComponent) {
+        throw new Error(
+          `There is already registered a component ${name} for the slot ${slot}. You can only register one slot component with no predicates per slot.`,
+        );
       }
+    }
 
-      const currentSlotComponent = currentSlot.data[name];
-      if (!currentSlot.slots.includes(name)) {
-        currentSlot.slots.push(name);
-      }
-      const slotComponentData = {
-        component,
-        predicates,
+    let currentSlot = this._data.slots[slot];
+    if (!currentSlot) {
+      this._data.slots[slot] = {
+        slots: [],
+        data: {},
       };
+      currentSlot = this._data.slots[slot];
+    }
+    if (!currentSlot.data[name]) {
+      currentSlot.data[name] = [];
+    }
 
-      // Try to set a displayName (useful for React dev tools) for the registered component
-      // Only if it's a function and it's not set previously
-      try {
-        const displayName = slotComponentData.component.displayName;
-
-        if (
-          !displayName &&
-          typeof slotComponentData?.component === 'function'
-        ) {
-          slotComponentData.component.displayName = name;
-        }
-      } catch (error) {
-        // eslint-disable-next-line no-console
-        console.warn(
-          `Not setting the slot component displayName because ${error}`,
-        );
+    const currentSlotComponent = currentSlot.data[name];
+    if (!currentSlot.slots.includes(name)) {
+      currentSlot.slots.push(name);
+    }
+    const slotComponentData = {
+      component,
+      predicates,
+    };
+
+    // Try to set a displayName (useful for React dev tools) for the registered component
+    // Only if it's a function and it's not set previously
+    try {
+      const displayName = slotComponentData.component.displayName;
+
+      if (!displayName && typeof slotComponentData?.component === 'function') {
+        slotComponentData.component.displayName = name;
       }
-
-      currentSlotComponent.push(slotComponentData);
+    } catch (error) {
+      // eslint-disable-next-line no-console
+      console.warn(
+        `Not setting the slot component displayName because ${error}`,
+      );
     }
+
+    currentSlotComponent.push(slotComponentData);
   }
 }
 

diff --git a/packages/registry/src/registry.test.tsx b/packages/registry/src/registry.test.tsx
@@ -113,7 +113,7 @@ describe('Component registry', () => {
   });
 });
 
-describe.only('Slots registry', () => {
+describe('Slots registry', () => {
   // config.slots.toolbar = [ // viewlets.xml
   //   'save',
   //   'edit',
@@ -148,6 +148,18 @@ describe.only('Slots registry', () => {
   // eslint-disable-next-line @typescript-eslint/no-unused-vars
   const ContentTypeConditionFalse = (contentType) => () => false;
 
+  it('registers a single slot component with no predicate', () => {
+    config.registerSlotComponent({
+      slot: 'toolbar',
+      name: 'save',
+      component: 'this is a toolbar component with no predicate',
+    });
+
+    expect(config.getSlot('toolbar', {})).toEqual([
+      'this is a toolbar component with no predicate',
+    ]);
+  });
+
   it('registers two slot components with predicates - registered components order is respected', () => {
     config.registerSlotComponent({
       slot: 'toolbar',
@@ -167,7 +179,7 @@ describe.only('Slots registry', () => {
       ],
     });
 
-    expect(config.getSlot('toolbar')).toEqual([
+    expect(config.getSlot('toolbar', {})).toEqual([
       'this is a toolbar component with only a truth-ish route condition',
     ]);
   });
@@ -217,6 +229,72 @@ describe.only('Slots registry', () => {
     expect(config.getSlot('toolbar', {})).toEqual([]);
   });
 
+  it('registers two slot components one without predicates - registered component with predicates are truthy, the last one registered wins', () => {
+    config.registerSlotComponent({
+      slot: 'toolbar',
+      name: 'save',
+      component: 'this is a toolbar component with no predicate',
+    });
+
+    config.registerSlotComponent({
+      slot: 'toolbar',
+      name: 'save',
+      component: 'this is a toolbar component with two truth-ish predicates',
+      predicates: [
+        RouteConditionTrue('/folder/path'),
+        ContentTypeConditionTrue(['News Item']),
+      ],
+    });
+
+    expect(config.getSlot('toolbar', {})).toEqual([
+      'this is a toolbar component with two truth-ish predicates',
+    ]);
+  });
+
+  it('registers two slot components one without predicates - registered components predicates are falsy, the one with no predicates wins', () => {
+    config.registerSlotComponent({
+      slot: 'toolbar',
+      name: 'save',
+      component: 'this is a toolbar component with no predicate',
+    });
+
+    config.registerSlotComponent({
+      slot: 'toolbar',
+      name: 'save',
+      component: 'this is a toolbar component with two truth-ish predicates',
+      predicates: [
+        RouteConditionFalse('/folder/path'),
+        ContentTypeConditionTrue(['News Item']),
+      ],
+    });
+
+    expect(config.getSlot('toolbar', {})).toEqual([
+      'this is a toolbar component with no predicate',
+    ]);
+  });
+
+  it('registers two slot components one without predicates - registered components predicates are truthy, the one with predicates wins', () => {
+    config.registerSlotComponent({
+      slot: 'toolbar',
+      name: 'save',
+      component: 'this is a toolbar component with two truth-ish predicates',
+      predicates: [
+        RouteConditionTrue('/folder/path'),
+        ContentTypeConditionTrue(['News Item']),
+      ],
+    });
+
+    config.registerSlotComponent({
+      slot: 'toolbar',
+      name: 'save',
+      component: 'this is a toolbar component with no predicate',
+    });
+
+    expect(config.getSlot('toolbar', {})).toEqual([
+      'this is a toolbar component with two truth-ish predicates',
+    ]);
+  });
+
   it('registers 2 + 2 slot components with predicates - No registered component have a truthy predicate', () => {
     config.registerSlotComponent({
       slot: 'toolbar',

diff --git a/packages/types/src/config/index.d.ts b/packages/types/src/config/index.d.ts
@@ -13,7 +13,7 @@ export type AddonRoutesConfig = {
 
 export type SlotComponent = {
   component: React.ComponentType<any>;
-  predicates: ((...args: any[]) => boolean)[];
+  predicates?: ((...args: any[]) => boolean)[];
 };
 
 export type SlotManager = {