♻️ Core+Types: resource-container-helper, clipboard, getChildJsonConf…

…ig, async-input (#35138)

* Move resource-container-helper to core/dom

* Remove allowlist entry for #preact/slot

* Define AmpElement extern and integrate PausableInterface

* Pass type-checking + some cleanup

* Use core mode for localdev check in preact/base-element

* Move clipboard to #core/window

* Fix types in clipboard

* Move getChildJsonConfig into #core/dom

* Code tweaks

* Move async-input under #core/constants

* Update imports of resource-container-helper

* Update imports of getChildJsonConfig

* Update imports of async input

* Comment-out cored externs from amp.extern.js

build-system/externs/amp.extern.js

@@ -467,7 +467,26 @@ let time;
  * Just an element, but used with AMP custom elements..
  * @constructor @extends {HTMLElement}
  */
-let AmpElement = function () {};
+
+// Commented out segments below have been migrated into
+// #core/dom/amp-element.extern, but are left here for now for easy access
+// during migration
+
+// let AmpElement = function () {};
+// /** */
+// AmpElement.prototype.pause = function () {};
+
+// /** */
+// AmpElement.prototype.unmount = function () {};
+
+// *
+//  * @param {number=} opt_parentPriority
+//  * @return {!Promise}
+
+// AmpElement.prototype.ensureLoaded = function (opt_parentPriority) {};
+
+// /** @return {?Element} */
+// AmpElement.prototype.getPlaceholder = function () {};
 
 /** @return {boolean} */
 AmpElement.prototype.R1 = function () {};
@@ -478,21 +497,6 @@ AmpElement.prototype.deferredMount = function () {};
 /** @return {!Signals} */
 AmpElement.prototype.signals = function () {};
 
-/** */
-AmpElement.prototype.pause = function () {};
-
-/** */
-AmpElement.prototype.unmount = function () {};
-
-/**
- * @param {number=} opt_parentPriority
- * @return {!Promise}
- */
-AmpElement.prototype.ensureLoaded = function (opt_parentPriority) {};
-
-/** @return {?Element} */
-AmpElement.prototype.getPlaceholder = function () {};
-
 /** @param {boolean} show */
 AmpElement.prototype.togglePlaceholder = function (show) {};
 

extensions/amp-analytics/0.1/config.js

@@ -20,7 +20,7 @@ import {assertHttpsUrl} from '../../../src/url';
 import {calculateScriptBaseUrl} from '#service/extension-script';
 import {deepMerge, dict, hasOwn} from '#core/types/object';
 import {dev, user, userAssert} from '../../../src/log';
-import {getChildJsonConfig} from '../../../src/json';
+import {getChildJsonConfig} from '#core/dom';
 import {getMode} from '../../../src/mode';
 import {isArray, isObject} from '#core/types';
 import {isCanary} from '#experiments';

extensions/amp-animation/0.1/amp-animation.js

@@ -22,7 +22,7 @@ import {WebAnimationPlayState} from './web-animation-types';
 import {WebAnimationService} from './web-animation-service';
 import {clamp} from '#core/math';
 import {dev, userAssert} from '../../../src/log';
-import {getChildJsonConfig} from '../../../src/json';
+import {getChildJsonConfig} from '#core/dom';
 import {getDetail, listen} from '../../../src/event-helper';
 import {installWebAnimationsIfNecessary} from './install-polyfill';
 import {isFiniteNumber} from '#core/types';

extensions/amp-consent/0.1/consent-config.js

@@ -21,7 +21,7 @@ import {Services} from '#service';
 import {childElementByTag} from '#core/dom/query';
 import {deepMerge, hasOwn, map} from '#core/types/object';
 import {devAssert, user, userAssert} from '../../../src/log';
-import {getChildJsonConfig} from '../../../src/json';
+import {getChildJsonConfig} from '#core/dom';
 
 const TAG = 'amp-consent/consent-config';
 const AMP_STORY_CONSENT_TAG = 'amp-story-consent';

extensions/amp-form/0.1/amp-form.js

@@ -20,7 +20,7 @@ import {AmpFormTextarea} from './amp-form-textarea';
 import {
   AsyncInputAttributes,
   AsyncInputClasses,
-} from '../../../src/async-input';
+} from '#core/constants/async-input';
 import {CSS} from '../../../build/amp-form-0.1.css';
 import {Deferred, tryResolve} from '#core/data-structures/promise';
 import {

extensions/amp-form/0.1/test/test-amp-form.js

@@ -28,7 +28,7 @@ import {AmpSelector} from '../../../amp-selector/0.1/amp-selector';
 import {
   AsyncInputAttributes,
   AsyncInputClasses,
-} from '../../../../src/async-input';
+} from '#core/constants/async-input';
 import {DIRTINESS_INDICATOR_CLASS} from '../form-dirtiness';
 import {Services} from '#service';
 import {cidServiceForDocForTesting} from '#service/cid-impl';

extensions/amp-lightbox/0.1/amp-lightbox.js

@@ -43,7 +43,7 @@ import {isInFie} from '../../../src/iframe-helper';
 import {realChildElements} from '#core/dom/query';
 import {toArray} from '#core/types/array';
 import {tryFocus} from '#core/dom';
-import {unmountAll} from '../../../src/utils/resource-container-helper';
+import {unmountAll} from '#core/dom/resource-container-helper';
 
 /** @const {string} */
 const TAG = 'amp-lightbox';

extensions/amp-lightbox/1.0/base-element.js

@@ -20,7 +20,7 @@ import {PreactBaseElement} from '#preact/base-element';
 import {dict} from '#core/types/object';
 import {toggle} from '#core/dom/style';
 import {toggleAttribute} from '#core/dom';
-import {unmountAll} from '../../../src/utils/resource-container-helper';
+import {unmountAll} from '#core/dom/resource-container-helper';
 
 export class BaseElement extends PreactBaseElement {
   /** @param {!AmpElement} element */

extensions/amp-link-rewriter/0.1/config-options.js

@@ -13,7 +13,7 @@
  * See the License for the specific language governing permissions and
  * limitations under the License.
  */
-import {getChildJsonConfig} from '../../../src/json';
+import {getChildJsonConfig} from '#core/dom';
 import {hasOwn} from '#core/types/object';
 import {user, userAssert} from '../../../src/log';
 

extensions/amp-recaptcha-input/0.1/amp-recaptcha-input.js

@@ -27,7 +27,7 @@ import {
 import {
   AsyncInputAttributes,
   AsyncInputClasses,
-} from '../../../src/async-input';
+} from '#core/constants/async-input';
 import {CSS} from '../../../build/amp-recaptcha-input-0.1.css';
 import {Layout} from '#core/dom/layout';
 import {setStyles, toggle} from '#core/dom/style';

extensions/amp-sidebar/0.1/amp-sidebar.js

@@ -42,7 +42,7 @@ import {removeFragment} from '../../../src/url';
 import {setModalAsClosed, setModalAsOpen} from '../../../src/modal';
 import {setStyles, toggle} from '#core/dom/style';
 import {toArray} from '#core/types/array';
-import {unmountAll} from '../../../src/utils/resource-container-helper';
+import {unmountAll} from '#core/dom/resource-container-helper';
 
 /** @private @const {string} */
 const TAG = 'amp-sidebar toolbar';

extensions/amp-sidebar/0.2/amp-sidebar.js

@@ -43,7 +43,7 @@ import {removeFragment} from '../../../src/url';
 import {setModalAsClosed, setModalAsOpen} from '../../../src/modal';
 import {setStyles, toggle} from '#core/dom/style';
 import {toArray} from '#core/types/array';
-import {unmountAll} from '../../../src/utils/resource-container-helper';
+import {unmountAll} from '#core/dom/resource-container-helper';
 
 /** @private @const {string} */
 const TAG = 'amp-sidebar toolbar';

extensions/amp-sidebar/1.0/base-element.js

@@ -19,7 +19,7 @@ import {CSS as COMPONENT_CSS} from './component.jss';
 import {PreactBaseElement} from '#preact/base-element';
 import {Sidebar} from './component';
 import {dict} from '#core/types/object';
-import {pauseAll} from '../../../src/utils/resource-container-helper';
+import {pauseAll} from '#core/dom/resource-container-helper';
 import {realChildNodes} from '#core/dom/query';
 import {toggle} from '#core/dom/style';
 import {toggleAttribute} from '#core/dom';

extensions/amp-skimlinks/0.1/skim-options.js

@@ -14,7 +14,7 @@
  * limitations under the License.
  */
 
-import {getChildJsonConfig} from '../../../src/json';
+import {getChildJsonConfig} from '#core/dom';
 import {getNormalizedHostnameFromUrl} from './utils';
 import {userAssert} from '../../../src/log';
 

extensions/amp-story/1.0/amp-story-request-service.js

@@ -15,7 +15,7 @@
  */
 
 import {Services} from '#service';
-import {getChildJsonConfig} from '../../../src/json';
+import {getChildJsonConfig} from '#core/dom';
 import {isProtocolValid} from '../../../src/url';
 import {once} from '#core/types/function';
 import {registerServiceBuilder} from '../../../src/service-helpers';

extensions/amp-story/1.0/amp-story-share.js

@@ -19,7 +19,7 @@ import {Toast} from './toast';
 import {
   copyTextToClipboard,
   isCopyingToClipboardSupported,
-} from '../../../src/clipboard';
+} from '#core/window/clipboard';
 import {dev, devAssert, user} from '../../../src/log';
 import {dict, map} from '#core/types/object';
 import {getLocalizationService} from './amp-story-localization-service';

extensions/amp-story/1.0/animation.js

@@ -35,7 +35,7 @@ import {
 import {assertDoesNotContainDisplay} from '../../../src/assert-display';
 import {dev, devAssert, user, userAssert} from '../../../src/log';
 import {escapeCssSelectorIdent} from '#core/dom/css-selectors';
-import {getChildJsonConfig} from '../../../src/json';
+import {getChildJsonConfig} from '#core/dom';
 import {map, omit} from '#core/types/object';
 import {prefersReducedMotion} from '#core/dom/media-query-props';
 import {

src/.eslintrc.js

@@ -60,7 +60,6 @@ module.exports = {
     {
       'files': [
         './preact/base-element.js',
-        './preact/slot.js',
         './polyfills/fetch.js',
         // TEMPORARY, follow tracking issue #33631
         './preact/component/3p-frame.js',

src/core/dom/video/pausable-interface.js → src/core/dom/amp-element.extern.js

@@ -14,11 +14,36 @@
  * limitations under the License.
  */
 
+/**
+ * @fileoverview Provides AmpElement interface for typechecking. Includes
+ * smaller interfaces that modules can type-check against to keep expected
+ * API surfaces narrow, so it's clear what properties and methods are
+ * expected/required.
+ *
+ * @externs
+ */
+
 /**
  * An interface for elements with pause functionality.
  * @interface
  */
-export class PausableInterface {
+class PausableInterface {
   /** @function */
   pause() {}
 }
+
+/**
+ * Just an element, but used with AMP custom elements..
+ * @constructor
+ * @extends {HTMLElement}
+ * @implements {PausableInterface}
+ */
+let AmpElement = function () {};
+
+/** @type {function(number=):!Promise}  */
+AmpElement.prototype.ensureLoaded;
+/** @type {function():Promise} */
+AmpElement.prototype.unmount;
+
+/** @type {function():?Element} */
+AmpElement.prototype.getPlaceholder;

src/core/dom/index.js

@@ -15,9 +15,10 @@
  */
 
 import {dict} from '#core/types/object';
+import {parseJson} from '#core/types/object/json';
 import {toWin} from '#core/window';
 
-import {matches} from './query';
+import {childElementsByTag, matches} from './query';
 
 const HTML_ESCAPE_CHARS = {
   '&': '&',
@@ -537,3 +538,29 @@ export function dispatchCustomEvent(node, name, opt_data, opt_options) {
 export function containsNotSelf(parent, child) {
   return child !== parent && parent.contains(child);
 }
+
+/**
+ * Helper method to get the json config from an element <script> tag
+ * @param {!Element} element
+ * @return {?JsonObject}
+ * @throws {!Error} If element does not have exactly one <script> child
+ * with type="application/json", or if the <script> contents are not valid JSON.
+ */
+export function getChildJsonConfig(element) {
+  const scripts = childElementsByTag(element, 'script');
+  const {length} = scripts;
+  if (length !== 1) {
+    throw new Error(`Found ${length} <script> children. Expected 1.`);
+  }
+
+  const script = scripts[0];
+  if (!isJsonScriptTag(script)) {
+    throw new Error('<script> child must have type="application/json"');
+  }
+
+  try {
+    return parseJson(script.textContent);
+  } catch {
+    throw new Error('Failed to parse <script> contents. Is it valid JSON?');
+  }
+}

src/utils/resource-container-helper.js → src/core/dom/resource-container-helper.js

@@ -15,6 +15,7 @@
  */
 
 import {tryCallback} from '#core/error';
+import {arrayOrSingleItemToArray} from '#core/types/array';
 
 const AMP_CLASS = 'i-amphtml-element';
 const DEEP = true;
@@ -67,17 +68,9 @@ export function forAllWithin(
   deep,
   callback
 ) {
-  if (Array.isArray(containerOrContainers)) {
-    for (let i = 0; i < containerOrContainers.length; i++) {
-      forAllWithinInternal(
-        containerOrContainers[i],
-        includeSelf,
-        deep,
-        callback
-      );
-    }
-  } else {
-    forAllWithinInternal(containerOrContainers, includeSelf, deep, callback);
+  const containers = arrayOrSingleItemToArray(containerOrContainers);
+  for (let i = 0; i < containers.length; i++) {
+    forAllWithinInternal(containers[i], includeSelf, deep, callback);
   }
 }
 
@@ -108,10 +101,13 @@ function forAllWithinInternal(container, includeSelf, deep, callback) {
     }
   }
 
-  const descendants = container.getElementsByClassName(AMP_CLASS);
+  const descendants =
+    /** @type {!HTMLCollection<!AmpElement>} */
+    (container.getElementsByClassName(AMP_CLASS));
+  /** @type {?Array<Element>} */
   let seen = null;
   for (let i = 0; i < descendants.length; i++) {
-    const descendant = /** @type {!AmpElement} */ (descendants[i]);
+    const descendant = descendants[i];
     if (deep) {
       // In deep search all elements will be covered.
       tryCallback(callback, descendant);

src/core/dom/video/pause-helper.js

@@ -19,29 +19,15 @@ import {
   unobserveBorderBoxSize,
 } from '#core/dom/layout/size-observer';
 
-import {PausableInterface} from './pausable-interface';
-
-/**
- * TODO(rcebulko, #34096): Remove this once a proper AMPElement type is
- * available, and declare that it @implements PausableInterface. For now, this
- * provides the necessary type strictness.
- * @extends Element
- * @implements {PausableInterface}
- */
-class PausableElementDef {
-  /** @function */
-  pause() {}
-}
-
 export class PauseHelper {
   /**
-   * @param {!PausableElementDef} element
+   * @param {!AmpElement} element
    */
   constructor(element) {
     /**
      * @private
      * @const
-     * @type {!PausableElementDef}
+     * @type {!AmpElement}
      */
     this.element_ = element;
 
@@ -84,7 +70,7 @@ export class PauseHelper {
     }
     this.hasSize_ = hasSize;
 
-    /** @implements {PausableInterface} */
+    /** @type {!PausableInterface} */
     const element = this.element_;
     if (!hasSize) {
       element.pause();

src/core/error/index.js

@@ -82,10 +82,11 @@ export function rethrowAsync(var_args) {
  * Executes the provided callback in a try/catch and rethrows any errors
  * asynchronously.
  *
- * @param {function(...*):T} callback
- * @param {...*} args
+ * @param {function(S):T} callback
+ * @param {S} args
  * @return {T}
  * @template T
+ * @template S
  */
 export function tryCallback(callback, ...args) {
   try {