diff --git a/extensions/amp-base-carousel/1.0/amp-base-carousel.js b/extensions/amp-base-carousel/1.0/amp-base-carousel.js index 77b47abe607da..498a00b36a67b 100644 --- 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 index 30c01d6ece4bf..690e43da6af1d 100644 --- 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}, + 'closeButtonAs': {selector: '[slot="close-button"]', single: true, as: true}, 'children': {passthrough: true}, }; diff --git a/extensions/amp-lightbox/1.0/storybook/Basic.amp.js b/extensions/amp-lightbox/1.0/storybook/Basic.amp.js index c76ca3abfabb1..4236cb938f3ac 100644 --- a/extensions/amp-lightbox/1.0/storybook/Basic.amp.js +++ b/extensions/amp-lightbox/1.0/storybook/Basic.amp.js @@ -46,9 +46,7 @@ export const Default = () => {

Test

- +
diff --git a/extensions/amp-stream-gallery/1.0/amp-stream-gallery.js b/extensions/amp-stream-gallery/1.0/amp-stream-gallery.js index 1b865e897586b..ad253c8758388 100644 --- 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 index 301501e6a40ce..8b3fe2bb42718 100644 --- a/src/preact/base-element.js +++ b/src/preact/base-element.js @@ -1082,7 +1082,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( @@ -1098,10 +1104,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 index dbe3b5a972c01..af01b98f452c7 100644 --- a/src/preact/slot.js +++ b/src/preact/slot.js @@ -24,21 +24,44 @@ import { pauseAll, unmountAll, } from '../utils/resource-container-helper'; +import {objectsEqualShallow} from '../core/types/object'; import {rediscoverChildren, removeProp, setProp} from '../context'; import {useAmpContext} from './context'; import {useEffect, useLayoutEffect, useRef} from './index'; const EMPTY = {}; +/** @const {WeakMap} */ +const cache = new WeakMap(); + /** * @param {!Element} element * @param {string} name - * @param {!Object|undefined} props - * @return {!PreactDef.VNode} + * @param {!Object|undefined} defaultProps + * @param {boolean|undefined} as + * @return {!PreactDef.VNode|!PreactDef.FunctionalComponent} */ -export function createSlot(element, name, props) { +export function createSlot(element, name, defaultProps, as = false) { element.setAttribute('slot', name); - return ; + if (!as) { + return ; + } + + const cached = cache.get(element); + if (cached && objectsEqualShallow(cached.oldProps, defaultProps)) { + return cached.component; + } + + /** + * @param {!Object|undefined} props + * @return {!PreactDef.VNode} + */ + function SlotWithProps(props) { + return ; + } + cache.set(element, {oldProps: defaultProps, component: SlotWithProps}); + + return SlotWithProps; } /** diff --git a/test/unit/preact/test-base-element-mapping.js b/test/unit/preact/test-base-element-mapping.js index 64164eaa402be..41dbdee7d14fd 100644 --- 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" >
+
{ ); }); + 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 new functional prop slot for "as" on mutation', async () => { + const {specialAs: prevComp} = lastProps; + const prevSpecial3 = prevComp(); + expect(prevSpecial3.props).to.deep.equal({ + valueWithDef: 'DEFAULT', + propA: 'A', + minFontSize: 72, + disabled: true, + name: 'i-amphtml-specialAs', + }); + + // Mutate slot prop, but this won't trigger a rerender + element + .querySelector('[special3]') + .setAttribute('value-with-def', 'CUSTOM'); + // Mutate an observed attr to trigger rerender + element.setAttribute('prop-a', 'B'); + + await waitFor(() => component.callCount > 1, 'component re-rendered'); + expect(component).to.be.calledTwice; + + const {specialAs: Comp} = lastProps; + expect(Comp).not.to.deep.equal(prevComp); + const special3 = Comp(); + expect(special3.props).to.deep.equal({ + valueWithDef: 'CUSTOM', + propA: 'A', + minFontSize: 72, + disabled: true, + name: 'i-amphtml-specialAs', + }); + }); + it('should pass children as prop slot array and parse attributes', () => { const {children} = lastProps; expect(children).to.have.lengthOf(2); @@ -1144,7 +1229,7 @@ describes.realWin('PreactBaseElement', spec, (env) => { it('should rerender on new children', async () => { await waitFor(() => component.callCount > 0, 'component rendered'); - const {children: prevChildren} = lastProps; + const {children: prevChildren, specialAs: prevSpecialAs} = lastProps; expect(prevChildren).to.have.lengthOf(1); const newChild = createElementWithAttributes(doc, 'div', { @@ -1157,7 +1242,7 @@ describes.realWin('PreactBaseElement', spec, (env) => { await waitFor(() => component.callCount > 1, 'component re-rendered'); expect(component).to.be.calledTwice; - const {children} = lastProps; + const {children, specialAs} = lastProps; expect(children).to.have.lengthOf(1); const child = children[0]; @@ -1170,6 +1255,7 @@ describes.realWin('PreactBaseElement', spec, (env) => { expect(element.querySelector('#child1').slot).to.equal(''); expect(element.querySelector('#child2').slot).to.equal(''); expect(element.textContent).to.contain('text (should be passed through)'); + expect(specialAs).to.deep.equal(prevSpecialAs); }); it('should rerender on text change', async () => { diff --git a/test/unit/preact/test-slot.js b/test/unit/preact/test-slot.js index 2e79c92ca835d..1a4763f50b11a 100644 --- a/test/unit/preact/test-slot.js +++ b/test/unit/preact/test-slot.js @@ -21,7 +21,7 @@ import { CanRender, LoadingProp, } from '../../../src/context/contextprops'; -import {Slot, useSlotContext} from '../../../src/preact/slot'; +import {Slot, createSlot, useSlotContext} from '../../../src/preact/slot'; import {WithAmpContext} from '../../../src/preact/context'; import {createElementWithAttributes} from '../../../src/dom'; import {createRef, useLayoutEffect, useRef} from '../../../src/preact'; @@ -344,3 +344,92 @@ describes.realWin('Slot mount/unmount', {}, (env) => { }); }); }); + +describes.realWin('createSlot', {}, (env) => { + let win, doc; + + beforeEach(() => { + win = env.win; + doc = win.document; + }); + + it('should create slot and set corresponding slot attr', () => { + const element = doc.createElement('div'); + const slot = createSlot(element, 'element'); + expect(element.getAttribute('slot')).to.equal('element'); + expect(slot.type).to.equal(Slot); + expect(slot.props).to.deep.equal({'name': 'element'}); + }); + + it('should create slot with props', () => { + const element = doc.createElement('div'); + const slot = createSlot(element, 'element', {'propA': true}); + expect(element.getAttribute('slot')).to.equal('element'); + expect(slot.type).to.equal(Slot); + expect(slot.props).to.deep.equal({'name': 'element', 'propA': true}); + }); + + it('should create slot function and set corresponding slot attr', () => { + const element = doc.createElement('div'); + const slotComp = createSlot(element, 'element', {}, /* as */ true); + expect(element.getAttribute('slot')).to.equal('element'); + + expect(typeof slotComp).to.equal('function'); + expect(slotComp.name).to.equal('SlotWithProps'); + + const slot = slotComp(); + expect(slot.type).to.equal(Slot); + expect(slot.props).to.deep.equal({'name': 'element'}); + }); + + it('should create slot function with props', () => { + const element = doc.createElement('div'); + const slotComp = createSlot( + element, + 'element', + {'propA': true}, + /* as */ true + ); + expect(element.getAttribute('slot')).to.equal('element'); + + expect(typeof slotComp).to.equal('function'); + expect(slotComp.name).to.equal('SlotWithProps'); + + const slot = slotComp(); + expect(slot.type).to.equal(Slot); + expect(slot.props).to.deep.equal({'name': 'element', 'propA': true}); + }); + + it('should return cached slot function', () => { + const element = doc.createElement('div'); + const slotComp = createSlot(element, 'element', {}, /* as */ true); + const slotComp2 = createSlot(element, 'element', {}, /* as */ true); + expect(slotComp2).to.deep.equal(slotComp); + }); + + it('should update cached slot function with new defaultProps', () => { + const element = doc.createElement('div'); + const slotComp = createSlot( + element, + 'element', + {'propA': false}, + /* as */ true + ); + const slotComp2 = createSlot( + element, + 'element', + {'propA': true}, + /* as */ true + ); + expect(slotComp2).not.to.deep.equal(slotComp); + }); + + it('should return unique slot functions for unique elements', () => { + const element = doc.createElement('div'); + const slotComp = createSlot(element, 'element', {}, /* as */ true); + + const element2 = doc.createElement('div'); + const slotComp2 = createSlot(element2, 'element', {}, /* as */ true); + expect(slotComp2).not.to.deep.equal(slotComp); + }); +});