♻️ Enable type-checking src/polyfills in CI

build-system/tasks/check-types.js

@@ -69,7 +69,15 @@ const PRIDE_FILES_GLOBS = [
   'node_modules/promise-pjs/promise.mjs',
 ];
 
-const CORE_EXTERNS_GLOB = 'src/core{,/**}/*.extern.js';
+// We provide glob lists for core src/externs since any other targets are
+// allowed to depend on core.
+const CORE_SRCS_GLOBS = [
+  'src/core/**/*.js',
+
+  // Needed for CSS escape polyfill
+  'third_party/css-escape/css-escape.js',
+];
+const CORE_EXTERNS_GLOBS = ['src/core/**/*.extern.js'];
 
 /**
  * Generates a list of source file paths for extensions to type-check
@@ -97,55 +105,56 @@ const TYPE_CHECK_TARGETS = {
   // To test a target locally:
   //   `amp check-types --target=src-foo-bar --warning_level=verbose`
   'src-amp-story-player': {
-    srcGlobs: ['src/amp-story-player{,/**}/*.js'],
+    srcGlobs: ['src/amp-story-player/**/*.js'],
     warningLevel: 'QUIET',
   },
   'src-context': {
-    srcGlobs: ['src/context{,/**}/*.js'],
+    srcGlobs: ['src/context/**/*.js'],
     warningLevel: 'QUIET',
   },
   'src-core': {
-    srcGlobs: [
-      'src/core{,/**}/*.js',
-      // Needed for CSS escape polyfill
-      'third_party/css-escape/css-escape.js',
-    ],
-    externGlobs: [CORE_EXTERNS_GLOB],
+    srcGlobs: CORE_SRCS_GLOBS,
+    externGlobs: CORE_EXTERNS_GLOBS,
   },
   'src-examiner': {
-    srcGlobs: ['src/examiner{,/**}/*.js'],
+    srcGlobs: ['src/examiner/**/*.js'],
     warningLevel: 'QUIET',
   },
   'src-experiments': {
-    srcGlobs: ['src/experiments{,/**}/*.js'],
+    srcGlobs: ['src/experiments/**/*.js'],
     warningLevel: 'QUIET',
   },
   'src-inabox': {
-    srcGlobs: ['src/inabox{,/**}/*.js'],
+    srcGlobs: ['src/inabox/**/*.js'],
     warningLevel: 'QUIET',
   },
   'src-polyfills': {
-    srcGlobs: ['src/polyfills{,/**}/*.js'],
-    warningLevel: 'QUIET',
+    srcGlobs: [
+      'src/polyfills/**/*.js',
+      // Exclude fetch its dependencies are cleaned up/extracted to core.
+      '!src/polyfills/fetch.js',
+      ...CORE_SRCS_GLOBS,
+    ],
+    externGlobs: ['src/polyfills/**/*.extern.js', ...CORE_EXTERNS_GLOBS],
   },
   'src-preact': {
-    srcGlobs: ['src/preact{,/**}/*.js'],
+    srcGlobs: ['src/preact/**/*.js'],
     warningLevel: 'QUIET',
   },
   'src-purifier': {
-    srcGlobs: ['src/purifier{,/**}/*.js'],
+    srcGlobs: ['src/purifier/**/*.js'],
     warningLevel: 'QUIET',
   },
   'src-service': {
-    srcGlobs: ['src/service{,/**}/*.js'],
+    srcGlobs: ['src/service/**/*.js'],
     warningLevel: 'QUIET',
   },
   'src-utils': {
-    srcGlobs: ['src/utils{,/**}/*.js'],
+    srcGlobs: ['src/utils/**/*.js'],
     warningLevel: 'QUIET',
   },
   'src-web-worker': {
-    srcGlobs: ['src/web-worker{,/**}/*.js'],
+    srcGlobs: ['src/web-worker/**/*.js'],
     warningLevel: 'QUIET',
   },
 
@@ -156,7 +165,7 @@ const TYPE_CHECK_TARGETS = {
   // bug for cherry-pick.
   'pride': {
     srcGlobs: PRIDE_FILES_GLOBS,
-    externGlobs: [CORE_EXTERNS_GLOB, 'build-system/externs/*.extern.js'],
+    externGlobs: ['build-system/externs/*.extern.js', ...CORE_EXTERNS_GLOBS],
   },
 
   // TODO(#33631): Targets below this point are not expected to pass.

src/.eslintrc.js

@@ -68,7 +68,10 @@ module.exports = {
       'rules': {'import/no-restricted-paths': isCiBuild() ? 0 : 1},
     },
     {
-      'files': ['./core/window.extern.js'],
+      'files': [
+        './core/window.extern.js',
+        './polyfills/custom-elements.extern.js',
+      ],
       'rules': {'local/no-global': 0},
     },
   ],

src/polyfills/abort-controller.js

@@ -14,40 +14,40 @@
  * limitations under the License.
  */
 
+/** Polyfill for the public AbortController. */
 class AbortController {
-  /** */
+  /** Constructor. */
   constructor() {
     /** @const {!AbortSignal} */
     this.signal_ = new AbortSignal();
   }
 
-  /** */
+  /** Triggers an abort signal. */
   abort() {
     if (this.signal_.isAborted_) {
       // Already aborted.
       return;
     }
     this.signal_.isAborted_ = true;
     if (this.signal_.onabort_) {
-      const event = {
+      const event = /** @type {!Event} */ ({
         'type': 'abort',
         'bubbles': false,
         'cancelable': false,
         'target': this.signal_,
         'currentTarget': this.signal_,
-      };
+      });
       this.signal_.onabort_(event);
     }
   }
 
-  /**
-   * @return {!AbortSignal}
-   */
+  /** @return {!AbortSignal} */
   get signal() {
     return this.signal_;
   }
 }
 
+/** Polyfill for the public AbortSignal. */
 class AbortSignal {
   /** */
   constructor() {
@@ -57,29 +57,24 @@ class AbortSignal {
     this.onabort_ = null;
   }
 
-  /**
-   * @return {boolean}
-   */
+  /** @return {boolean} */
   get aborted() {
     return this.isAborted_;
   }
 
-  /**
-   * @return {?function(!Event)}
-   */
+  /** @return {?function(!Event)} */
   get onabort() {
     return this.onabort_;
   }
 
-  /**
-   * @param {?function(!Event)} value
-   */
+  /** @param {?function(!Event)} value */
   set onabort(value) {
     this.onabort_ = value;
   }
 }
 
 /**
+ * Sets the AbortController and AbortSignal polyfills if not defined.
  * @param {!Window} win
  */
 export function install(win) {

src/polyfills/custom-elements.extern.js

@@ -0,0 +1,29 @@
+/**
+ * Copyright 2021 The AMP HTML Authors. All Rights Reserved.
+ *
+ * Licensed under the Apache License, Version 2.0 (the "License");
+ * you may not use this file except in compliance with the License.
+ * You may obtain a copy of the License at
+ *
+ *      http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS-IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+// This is needed only by custom elements, so it doesn't need to be in the core
+// window externs file.
+/** @type {!typeof HTMLElement} */
+window.HTMLElementOrig;
+
+// These callbacks are used for custom elements, but don't appear to belong to
+// any available type definitions. There's a TODO(jridgewell) from ~2 years ago
+// to call them differently; if that can be figured out, these may be dropped.
+// If not, these may belong to a separate node.extern.js externs file.
+/** @function @this {Node} */
+Node.prototype.connectedCallback;
+/** @function @this {Node} */
+Node.prototype.disconnectedCallback;