Merge branch 'issue-7-fix-nested-slot-issues'

Author: patricknelson

index.js

@@ -48,11 +48,14 @@ function createSvelteSlots(slots) {
  * @param {string[]?} opts.attributes Optional array of attributes that should be reactively forwarded to the component when modified.
  * @param {boolean?}  opts.shadow     Indicates if we should build the component in the shadow root instead of in the regular ("light") DOM.
  * @param {string?}   opts.href       URL to the CSS stylesheet to incorporate into the shadow DOM (if enabled).
+ * @param {boolean?}  opts.debugMode  Hidden option to enable debugging for package development purposes.
  */
 export default function(opts) {
 	class Wrapper extends HTMLElement {
 		constructor() {
 			super();
+
+			this.debug('constructor()');
 			this.slotCount = 0;
 			let root = opts.shadow ? this.attachShadow({ mode: 'open' }) : this;
 
@@ -77,6 +80,8 @@ export default function(opts) {
 		}
 
 		connectedCallback() {
+			this.debug('connectedCallback()');
+
 			// Props passed to Svelte component constructor.
 			let props = {
 				$$scope: {}
@@ -86,37 +91,52 @@ export default function(opts) {
 
 			// Populate custom element attributes into the props object.
 			// TODO: Inspect component and normalize to lowercase for Lit-style props (https://github.com/crisward/svelte-tag/issues/16)
-			let slots;
 			Array.from(this.attributes).forEach(attr => props[attr.name] = attr.value);
 
+			// Setup slot elements, making sure to retain a reference to the original elements prior to processing, so they
+			// can be restored later on disconnectedCallback().
+			this.slotEls = {};
 			if (opts.shadow) {
-				slots = this.getShadowSlots();
+				this.slotEls = this.getShadowSlots();
 				this.observer = new MutationObserver(this.processMutations.bind(this, { root: this._root, props }));
 				this.observer.observe(this, { childList: true, subtree: true, attributes: false });
 			} else {
-				slots = this.getSlots();
+				this.slotEls = this.getSlots();
 			}
-			this.slotCount = Object.keys(slots).length;
-			props.$$slots = createSvelteSlots(slots);
+			this.slotCount = Object.keys(this.slotEls).length; // TODO: Refactor to getter
+			props.$$slots = createSvelteSlots(this.slotEls);
 
 			this.elem = new opts.component({ target: this._root, props });
 		}
 
 		disconnectedCallback() {
+			this.debug('disconnectedCallback()');
+
 			if (this.observer) {
 				this.observer.disconnect();
 			}
 
-			// Double check that element has been initialized already. This could happen in case connectedCallback (which
-			// is async) hasn't fully completed yet.
+			// Double check that element has been initialized already. This could happen in case connectedCallback() hasn't
+			// fully completed yet (e.g. if initialization is async) TODO: May happen later if MutationObserver is setup for light DOM
 			if (this.elem) {
 				try {
-					// destroy svelte element when removed from domn
+					// Clean up: Destroy Svelte component when removed from DOM.
 					this.elem.$destroy();
 				} catch(err) {
 					console.error(`Error destroying Svelte component in '${this.tagName}'s disconnectedCallback(): ${err}`);
 				}
 			}
+
+			if (!opts.shadow) {
+				// Go through originally removed slots and restore back to the custom element. This is necessary in case
+				// we're just being appended elsewhere in the DOM (likely if we're nested under another custom element
+				// that initializes after this custom element, thus causing *another* round of construct/connectedCallback
+				// on this one).
+				for(let slotName in this.slotEls) {
+					let slotEl = this.slotEls[slotName];
+					this.appendChild(slotEl);
+				}
+			}
 		}
 
 		/**
@@ -135,17 +155,64 @@ export default function(opts) {
 			return node;
 		}
 
+		/**
+		 * Traverses DOM to find the first custom element that the provided <slot> element happens to belong to.
+		 *
+		 * @param {Element} slot
+		 * @returns {HTMLElement|null}
+		 */
+		findSlotParent(slot) {
+			let parentEl = slot.parentElement;
+			while(parentEl) {
+				if (parentEl.tagName.indexOf('-') !== -1) return parentEl;
+				parentEl = parentEl.parentElement;
+			}
+			return null;
+		}
+
+		/**
+		 * Indicates if the provided <slot> element instance belongs to this custom element or not.
+		 *
+		 * @param {Element} slot
+		 * @returns {boolean}
+		 */
+		isOwnSlot(slot) {
+			let slotParent = this.findSlotParent(slot);
+			if (slotParent === null) return false;
+			return (slotParent === this);
+		}
+
 		getSlots() {
-			const namedSlots = this.querySelectorAll('[slot]');
 			let slots = {};
-			namedSlots.forEach(n => {
-				slots[n.slot] = n;
-				this.removeChild(n);
-			});
-			if (this.innerHTML.length) {
-				slots.default = this.unwrap(this);
+
+			// Look for named slots below this element. IMPORTANT: This may return slots nested deeper (see check in forEach below).
+			const queryNamedSlots = this.querySelectorAll('[slot]');
+			for(let candidate of queryNamedSlots) {
+				// Skip this slot if it doesn't happen to belong to THIS custom element.
+				if (!this.isOwnSlot(candidate)) continue;
+
+				slots[candidate.slot] = candidate;
+				// TODO: Potentially problematic in edge cases where the browser may *oddly* return slots from query selector
+				//  above, yet their not actually a child of the current element. This seems to only happen if another
+				//  constructor() + connectedCallback() are BOTH called for this particular element again BEFORE a
+				//  disconnectedCallback() gets called (out of sync). Only experienced in Chrome when manually editing the HTML
+				//  when there were multiple other custom elements present inside the slot of another element (very edge case?)
+				this.removeChild(candidate);
+			}
+
+			// Default slots are indeed allowed alongside named slots, as long as the named slots are elided *first*. We
+			// should also make sure we trim out whitespace in case all slots and elements are already removed. We don't want
+			// to accidentally pass content (whitespace) to a component that isn't setup with a default slot.
+			if (this.innerHTML.trim().length !== 0) {
+				if (slots.default) {
+					// Edge case: User has a named "default" as well as remaining HTML left over. Use same error as Svelte.
+					console.error(`svelteRetag: '${this.tagName}': Found elements without slot attribute when using slot="default"`);
+				} else {
+					slots.default = this.unwrap(this);
+				}
 				this.innerHTML = '';
 			}
+
 			return slots;
 		}
 
@@ -168,12 +235,22 @@ export default function(opts) {
 		//  light DOM, since that is not deferred and technically slots will be added after the wrapping tag's connectedCallback()
 		//  during initial browser parsing and before the closing tag is encountered.
 		processMutations({ root, props }, mutations) {
+			this.debug('processMutations()');
+
 			for(let mutation of mutations) {
 				if (mutation.type === 'childList') {
 					let slots = this.getShadowSlots();
+
+					// TODO: Should it re-render if the count changes at all? e.g. what if slots were REMOVED (reducing it to zero)?
+					//  We'd have latent content left over that's not getting updated, then. Consider rewrite...
 					if (Object.keys(slots).length) {
+
 						props.$$slots = createSvelteSlots(slots);
+
+						// TODO: Why is this set here but not re-rendered unless the slot count changes?
+						// TODO: Also, why is props.$$slots set above but not just passed here? Calling createSvelteSlots(slots) 2x...
 						this.elem.$set({ '$$slots': createSvelteSlots(slots) });
+
 						// do full re-render on slot count change - needed for tabs component
 						if (this.slotCount !== Object.keys(slots).length) {
 							Array.from(this.attributes).forEach(attr => props[attr.name] = attr.value); // TODO: Redundant, repeated on connectedCallback().
@@ -186,11 +263,32 @@ export default function(opts) {
 			}
 		}
 
+		/**
+		 * Forward modifications to element attributes to the corresponding Svelte prop.
+		 *
+		 * @param {string} name
+		 * @param {string} oldValue
+		 * @param {string} newValue
+		 */
 		attributeChangedCallback(name, oldValue, newValue) {
+			this.debug('attributes changed', { name, oldValue, newValue });
+
 			if (this.elem && newValue !== oldValue) {
 				this.elem.$set({ [name]: newValue });
 			}
 		}
+
+		/**
+		 * Pass through to console.log() but includes a reference to the custom element in the log for easier targeting for
+		 * debugging purposes.
+		 *
+		 * @param {...*}
+		 */
+		debug() {
+			if (opts.debugMode) {
+				console.log.apply(null, [this, ...arguments]);
+			}
+		}
 	}
 
 	window.customElements.define(opts.tagname, Wrapper);