Support "as" with "selector" prop defs

ampproject · Apr 21, 2021 · 449912e · 449912e
1 parent 658a563
commit 449912e
Show file tree

Hide file tree

Showing 6 changed files with 83 additions and 40 deletions.
diff --git a/extensions/amp-base-carousel/1.0/amp-base-carousel.js b/extensions/amp-base-carousel/1.0/amp-base-carousel.js
@@ -21,7 +21,6 @@ import {CSS} from '../../../build/amp-base-carousel-1.0.css';
 import {CarouselContextProp} from './carousel-props';
 import {PreactBaseElement} from '../../../src/preact/base-element';
 import {Services} from '../../../src/services';
-import {cloneElement} from '../../../src/preact';
 import {createCustomEvent} from '../../../src/event-helper';
 import {dict} from '../../../src/core/types/object';
 import {dispatchCustomEvent} from '../../../src/dom';
@@ -85,17 +84,6 @@ class AmpBaseCarousel extends PreactBaseElement {
       this.api().goToSlide(slide);
     }
   }
-
-  /** @override */
-  updatePropsForRendering(props) {
-    const {arrowPrev, arrowNext} = props;
-    if (arrowPrev) {
-      props['arrowPrevAs'] = (props) => cloneElement(arrowPrev, props);
-    }
-    if (arrowNext) {
-      props['arrowNextAs'] = (props) => cloneElement(arrowNext, props);
-    }
-  }
 }
 
 /** @override */
@@ -107,13 +95,15 @@ AmpBaseCarousel['layoutSizeDefined'] = true;
 /** @override */
 AmpBaseCarousel['props'] = {
   'advanceCount': {attr: 'advance-count', type: 'number', media: true},
-  'arrowPrev': {
+  'arrowPrevAs': {
     selector: '[slot="prev-arrow"]',
     single: true,
+    as: true,
   },
-  'arrowNext': {
+  'arrowNextAs': {
     selector: '[slot="next-arrow"]',
     single: true,
+    as: true,
   },
   'autoAdvance': {attr: 'auto-advance', type: 'boolean', media: true},
   'autoAdvanceCount': {attr: 'auto-advance-count', type: 'number', media: true},

diff --git a/extensions/amp-lightbox/1.0/base-element.js b/extensions/amp-lightbox/1.0/base-element.js
@@ -45,11 +45,6 @@ export class BaseElement extends PreactBaseElement {
     this.removeAsContainer();
   }
 
-  /** @override */
-  updatePropsForRendering(props) {
-    props['closeButtonAs'] = () => props['closeButton'];
-  }
-
   /** @private */
   beforeOpen_() {
     this.open_ = true;
@@ -95,7 +90,7 @@ BaseElement['Component'] = Lightbox;
 /** @override */
 BaseElement['props'] = {
   'animation': {attr: 'animation', media: true, default: 'fade-in'},
-  'closeButton': {selector: '[slot="close-button"]', single: true},
+  'closeButton': {selector: '[slot="close-button"]', single: true, as: true},
   'children': {passthrough: true},
 };
 

diff --git a/extensions/amp-stream-gallery/1.0/amp-stream-gallery.js b/extensions/amp-stream-gallery/1.0/amp-stream-gallery.js
@@ -21,7 +21,6 @@ import {CSS as GALLERY_CSS} from './stream-gallery.jss';
 import {PreactBaseElement} from '../../../src/preact/base-element';
 import {Services} from '../../../src/services';
 import {StreamGallery} from './stream-gallery';
-import {cloneElement} from '../../../src/preact';
 import {createCustomEvent} from '../../../src/event-helper';
 import {dict} from '../../../src/core/types/object';
 import {dispatchCustomEvent} from '../../../src/dom';
@@ -62,17 +61,6 @@ class AmpStreamGallery extends PreactBaseElement {
     );
     return super.isLayoutSupported(layout);
   }
-
-  /** @override */
-  updatePropsForRendering(props) {
-    const {arrowPrev, arrowNext} = props;
-    if (arrowPrev) {
-      props['arrowPrevAs'] = (props) => cloneElement(arrowPrev, props);
-    }
-    if (arrowNext) {
-      props['arrowNextAs'] = (props) => cloneElement(arrowNext, props);
-    }
-  }
 }
 
 /**
@@ -109,13 +97,15 @@ AmpStreamGallery['layoutSizeDefined'] = true;
 
 /** @override */
 AmpStreamGallery['props'] = {
-  'arrowPrev': {
+  'arrowPrevAs': {
     selector: '[slot="prev-arrow"]',
     single: true,
+    as: true,
   },
-  'arrowNext': {
+  'arrowNextAs': {
     selector: '[slot="next-arrow"]',
     single: true,
+    as: true,
   },
   'controls': {attr: 'controls', type: 'string', media: true},
   'extraSpace': {attr: 'extra-space', type: 'string', media: true},

diff --git a/src/preact/base-element.js b/src/preact/base-element.js
@@ -1081,7 +1081,13 @@ function parsePropDefs(Ctor, props, propDefs, element, mediaQueryProps) {
         continue;
       }
       const def = propDefs[match];
-      const {single, name = match, clone, props: slotProps = {}} = def;
+      const {
+        as = false,
+        single,
+        name = match,
+        clone,
+        props: slotProps = {},
+      } = def;
       devAssert(clone || Ctor['usesShadowDom']);
       const parsedSlotProps = {};
       parsePropDefs(
@@ -1097,10 +1103,12 @@ function parsePropDefs(Ctor, props, propDefs, element, mediaQueryProps) {
         props[name] = createSlot(
           childElement,
           childElement.getAttribute('slot') || `i-amphtml-${name}`,
-          parsedSlotProps
+          parsedSlotProps,
+          as
         );
       } else {
         const list = props[name] || (props[name] = []);
+        devAssert(!as);
         list.push(
           clone
             ? createShallowVNodeCopy(childElement)

diff --git a/src/preact/slot.js b/src/preact/slot.js
@@ -32,12 +32,20 @@ const EMPTY = {};
 /**
  * @param {!Element} element
  * @param {string} name
- * @param {!Object|undefined} props
- * @return {!PreactDef.VNode}
+ * @param {!Object|undefined} defaultProps
+ * @param {boolean} as
+ * @return {!PreactDef.VNode|!PreactDef.FunctionalComponent}
  */
-export function createSlot(element, name, props) {
+export function createSlot(element, name, defaultProps, as) {
+  /**
+   * @param {!Object|undefined} props
+   * @return {!PreactDef.VNode}
+   */
+  function SlotWithProps(props) {
+    return <Slot {...(defaultProps || EMPTY)} name={name} {...props} />;
+  }
   element.setAttribute('slot', name);
-  return <Slot {...(props || EMPTY)} name={name} />;
+  return as ? SlotWithProps : SlotWithProps();
 }
 
 /**

diff --git a/test/unit/preact/test-base-element-mapping.js b/test/unit/preact/test-base-element-mapping.js
@@ -561,6 +561,19 @@ describes.realWin('PreactBaseElement', spec, (env) => {
           selector: '[special2]',
           single: true,
         },
+        'specialAs': {
+          selector: '[special3]',
+          props: {
+            'noValue': {attr: 'no-value'},
+            'valueWithDef': {attr: 'value-with-def', default: 'DEFAULT'},
+            'propA': {attr: 'prop-a'},
+            'minFontSize': {attr: 'min-font-size', type: 'number'},
+            'disabled': {attr: 'disabled', type: 'boolean'},
+            'enabled': {attr: 'enabled', type: 'boolean'},
+          },
+          single: true,
+          as: true,
+        },
         'children': {
           props: {
             'boolDefTrue': {
@@ -589,6 +602,13 @@ describes.realWin('PreactBaseElement', spec, (env) => {
             disabled
             unknown="1"
           ></div>
+          <div
+            special3
+            prop-a="A"
+            min-font-size="72"
+            disabled
+            unknown="1"
+          ></div>
           <div
             id="child1"
             part-a="A"
@@ -648,6 +668,38 @@ describes.realWin('PreactBaseElement', spec, (env) => {
       );
     });
 
+    it('should pass children as functional prop slot for single-element mapping with "as" and parse attributes', () => {
+      const {specialAs: Comp} = lastProps;
+      expect(typeof Comp).to.equal('function');
+      expect(Comp.name).to.equal('SlotWithProps');
+
+      const special3 = Comp();
+      expect(special3.props).to.deep.equal({
+        valueWithDef: 'DEFAULT',
+        propA: 'A',
+        minFontSize: 72,
+        disabled: true,
+        name: 'i-amphtml-specialAs',
+      });
+
+      const special3WithProps = Comp({
+        'aria-disabled': 'false',
+        disabled: false,
+      });
+      expect(special3WithProps.props).to.deep.equal({
+        valueWithDef: 'DEFAULT',
+        propA: 'A',
+        minFontSize: 72,
+        name: 'i-amphtml-specialAs',
+        'aria-disabled': 'false',
+        disabled: false,
+      });
+
+      expect(element.querySelector('[special3]').slot).to.equal(
+        'i-amphtml-specialAs'
+      );
+    });
+
     it('should pass children as prop slot array and parse attributes', () => {
       const {children} = lastProps;
       expect(children).to.have.lengthOf(2);