From 21390e7a48139c077ecca2791e0772f31f82f7ff Mon Sep 17 00:00:00 2001 From: Ryan Cebulko Date: Wed, 5 May 2021 11:59:47 -0400 Subject: [PATCH 01/21] Clean up directory globs --- build-system/tasks/check-types.js | 27 ++++++++++++++------------- 1 file changed, 14 insertions(+), 13 deletions(-) diff --git a/build-system/tasks/check-types.js b/build-system/tasks/check-types.js index 6f8376682fd3..fea16cd0b267 100644 --- a/build-system/tasks/check-types.js +++ b/build-system/tasks/check-types.js @@ -69,7 +69,8 @@ const PRIDE_FILES_GLOBS = [ 'node_modules/promise-pjs/promise.mjs', ]; -const CORE_EXTERNS_GLOB = 'src/core{,/**}/*.extern.js'; +const CORE_SRCS_GLOB = 'src/core/**/*.js'; +const CORE_EXTERNS_GLOB = 'src/core/**/*.extern.js'; /** * Generates a list of source file paths for extensions to type-check @@ -97,55 +98,55 @@ 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', + CORE_SRCS_GLOB, // Needed for CSS escape polyfill 'third_party/css-escape/css-escape.js', ], externGlobs: [CORE_EXTERNS_GLOB], }, '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'], + srcGlobs: ['src/polyfills/**/*.js'], warningLevel: 'QUIET', }, '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', }, From 4a25a3b66a1fa4c591679d017c42accd139b7fb2 Mon Sep 17 00:00:00 2001 From: Ryan Cebulko Date: Wed, 5 May 2021 12:09:06 -0400 Subject: [PATCH 02/21] Move src/core srcs+externs to const arrays --- build-system/tasks/check-types.js | 25 ++++++++++++++----------- 1 file changed, 14 insertions(+), 11 deletions(-) diff --git a/build-system/tasks/check-types.js b/build-system/tasks/check-types.js index fea16cd0b267..ab475a599764 100644 --- a/build-system/tasks/check-types.js +++ b/build-system/tasks/check-types.js @@ -69,8 +69,15 @@ const PRIDE_FILES_GLOBS = [ 'node_modules/promise-pjs/promise.mjs', ]; -const CORE_SRCS_GLOB = 'src/core/**/*.js'; -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 @@ -106,12 +113,8 @@ const TYPE_CHECK_TARGETS = { warningLevel: 'QUIET', }, 'src-core': { - srcGlobs: [ - CORE_SRCS_GLOB, - // 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'], @@ -126,8 +129,8 @@ const TYPE_CHECK_TARGETS = { warningLevel: 'QUIET', }, 'src-polyfills': { - srcGlobs: ['src/polyfills/**/*.js'], - warningLevel: 'QUIET', + srcGlobs: ['src/polyfills/**/*.js', ...CORE_SRCS_GLOBS], + externGlobs: CORE_EXTERNS_GLOBS, }, 'src-preact': { srcGlobs: ['src/preact/**/*.js'], @@ -157,7 +160,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. From 68b36cef878dacb50f133d646627d9f16931ec19 Mon Sep 17 00:00:00 2001 From: Ryan Cebulko Date: Wed, 5 May 2021 12:33:49 -0400 Subject: [PATCH 03/21] Exclude fetch/get-bounding-client-rect for now --- build-system/tasks/check-types.js | 10 ++++++-- src/polyfills/shame.extern.js | 40 +++++++++++++++++++++++++++++++ 2 files changed, 48 insertions(+), 2 deletions(-) create mode 100644 src/polyfills/shame.extern.js diff --git a/build-system/tasks/check-types.js b/build-system/tasks/check-types.js index ab475a599764..d8eb31d7f060 100644 --- a/build-system/tasks/check-types.js +++ b/build-system/tasks/check-types.js @@ -129,8 +129,14 @@ const TYPE_CHECK_TARGETS = { warningLevel: 'QUIET', }, 'src-polyfills': { - srcGlobs: ['src/polyfills/**/*.js', ...CORE_SRCS_GLOBS], - externGlobs: CORE_EXTERNS_GLOBS, + srcGlobs: [ + 'src/polyfills/**/*.js', + // Exclude fetch and get-bounding-client-rect until their dependencies are + // cleaned up/extracted to core. + '!src/polyfills/{get-bounding-client-rect,fetch}.js', + ...CORE_SRCS_GLOBS, + ], + externGlobs: ['src/polyfills/**/*.extern.js', ...CORE_EXTERNS_GLOBS], }, 'src-preact': { srcGlobs: ['src/preact/**/*.js'], diff --git a/src/polyfills/shame.extern.js b/src/polyfills/shame.extern.js new file mode 100644 index 000000000000..198c5f72ab23 --- /dev/null +++ b/src/polyfills/shame.extern.js @@ -0,0 +1,40 @@ +/** + * 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. + */ + +/** + * @fileoverview The junk-drawer of externs that haven't yet been sorted well. + * Shame! Shame! Shame! Avoid adding to this. + * + * It's okay for some things to start off here, since moving them doesn't + * require any other file changes (unlike real code, which requires updating) + * imports throughout the repo). + * + * @externs + */ + +/** + * These definitions are exported by the fetch and get-bounding-client-rect + * polyfill implementation files, but currently they have dependencies on + * non-core modules that aren't yet type-checked. + * + * Planned destination: These externs should be * removed when those files are + * re-included in the polyfills type-check target. + */ + +/** @type {function(!Window)} */ +let install$$module$src$polyfills$fetch; +/** @type {function(!Window)} */ +let install$$module$src$polyfills$get_bounding_client_rect; From 6426568f50869938978df363c137557794d0811d Mon Sep 17 00:00:00 2001 From: Ryan Cebulko Date: Wed, 5 May 2021 12:34:03 -0400 Subject: [PATCH 04/21] Provide extern for promise-pjs polyfill --- src/polyfills/promise.extern.js | 23 +++++++++++++++++++++++ 1 file changed, 23 insertions(+) create mode 100644 src/polyfills/promise.extern.js diff --git a/src/polyfills/promise.extern.js b/src/polyfills/promise.extern.js new file mode 100644 index 000000000000..82d7de4b6a17 --- /dev/null +++ b/src/polyfills/promise.extern.js @@ -0,0 +1,23 @@ +/** + * 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. + */ + +/** + * @fileoverview Provide extern for promise-pjs polyfill. + * @externs + */ + +/** type {!Promise} */ +let default$$module$promise_pjs; From 3750054772a8c079b86f8081559755b2bed0d32f Mon Sep 17 00:00:00 2001 From: Ryan Cebulko Date: Wed, 5 May 2021 12:34:21 -0400 Subject: [PATCH 05/21] Fix types and add comments in abort-controller --- src/polyfills/abort-controller.js | 27 +++++++++++---------------- 1 file changed, 11 insertions(+), 16 deletions(-) diff --git a/src/polyfills/abort-controller.js b/src/polyfills/abort-controller.js index 6127f8a6516b..4f0e6f54278f 100644 --- a/src/polyfills/abort-controller.js +++ b/src/polyfills/abort-controller.js @@ -14,14 +14,15 @@ * 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. @@ -29,25 +30,24 @@ class AbortController { } 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) { From c05199f8961e683e2a6436ee6ae67af55ca4d6f9 Mon Sep 17 00:00:00 2001 From: Ryan Cebulko Date: Wed, 5 May 2021 14:08:31 -0400 Subject: [PATCH 06/21] Fix types and add comments in intersection-observer-stub --- .../stubs/intersection-observer-stub.js | 76 ++++++++++--------- 1 file changed, 41 insertions(+), 35 deletions(-) diff --git a/src/polyfills/stubs/intersection-observer-stub.js b/src/polyfills/stubs/intersection-observer-stub.js index baca8160e620..6da62e9b49cf 100644 --- a/src/polyfills/stubs/intersection-observer-stub.js +++ b/src/polyfills/stubs/intersection-observer-stub.js @@ -23,6 +23,9 @@ * amp-intersection-observer-polyfill extension. */ +/** @typedef {typeof IntersectionObserver} */ +let IntersectionObserverCtor; + const UPGRADERS = '_upgraders'; const NATIVE = '_native'; const STUB = '_stub'; @@ -52,23 +55,30 @@ export function shouldLoadPolyfill(win) { * @return {boolean} */ function isWebkit(win) { - // navigator.vendor is always "Apple Computer, Inc." for all iOS browsers and Mac OS Safari. + // navigator.vendor is always "Apple Computer, Inc." for all iOS browsers and + // Mac OS Safari. return /apple/i.test(win.navigator.vendor); } /** - * @param {typeof IntersectionObserver} Native - * @param {typeof IntersectionObserver} Polyfill - * @return {typeof IntersectionObserver} + * @param {IntersectionObserverCtor} Native + * @param {IntersectionObserverCtor} Polyfill + * @return {IntersectionObserverCtor} */ function getIntersectionObserverDispatcher(Native, Polyfill) { - return function (ioCallback, opts) { - if (opts && opts.root && opts.root.nodeType === 9) { + /** + * @param {!IntersectionObserverCallback} ioCallback + * @param {IntersectionObserverInit=} opts + * @return {!IntersectionObserver} + */ + function Ctor(ioCallback, opts) { + if (opts?.root?.nodeType === /* Node.DOCUMENT_NODE */ 9) { return new Polyfill(ioCallback, opts); } else { return new Native(ioCallback, opts); } - }; + } + return Ctor; } /** @@ -101,7 +111,11 @@ export function installStub(win) { */ export function supportsDocumentRoot(win) { try { - new win.IntersectionObserver(() => {}, {root: win.document}); + new win.IntersectionObserver(() => {}, { + // TODO(rcebulko): Update when CC updates their externs + // See https://github.com/google/closure-compiler/pull/3804 + root: /** @type {?} */ (win.document), + }); return true; } catch { return false; @@ -140,7 +154,7 @@ export function upgradePolyfill(win, installer) { } Stub[ UPGRADERS - ] = /** @type {!Array} */ ({ + ] = /** @type {!Array} */ ({ 'push': upgrade, }); } else { @@ -154,6 +168,10 @@ export function upgradePolyfill(win, installer) { * The stub for `IntersectionObserver`. Implements the same interface, but * keeps the tracked elements in memory until the actual polyfill arives. * This stub is necessary because the polyfill itself is significantly bigger. + * + * It doesn't technically extend IntersectionObserver, but this allows the stub + * to be seen as equivalent when typechecking calls expecting an IntObs. + * @extends IntersectionObserver */ export class IntersectionObserverStub { /** @@ -181,9 +199,7 @@ export class IntersectionObserverStub { IntersectionObserverStub[UPGRADERS].push(this.upgrade_.bind(this)); } - /** - * @return {?Element} - */ + /** @return {?Element} */ get root() { if (this.inst_) { return this.inst_.root; @@ -191,19 +207,18 @@ export class IntersectionObserverStub { return this.options_.root || null; } - /** - * @return {*} - */ + /** @return {string} */ get rootMargin() { if (this.inst_) { return this.inst_.rootMargin; } - return this.options_.rootMargin; + // The CC-provided IntersectionObserverInit type allows for rootMargin to be + // undefined, but we provide a default, so it's guaranteed to be a string + // here. + return /** @type {string} */ (this.options_.rootMargin); } - /** - * @return {*} - */ + /** @return {!Array} */ get thresholds() { if (this.inst_) { return this.inst_.thresholds; @@ -211,9 +226,6 @@ export class IntersectionObserverStub { return [].concat(this.options_.threshold || 0); } - /** - * @return {undefined} - */ disconnect() { if (this.inst_) { this.inst_.disconnect(); @@ -222,9 +234,7 @@ export class IntersectionObserverStub { } } - /** - * @return {!Array} - */ + /** @return {!Array} */ takeRecords() { if (this.inst_) { return this.inst_.takeRecords(); @@ -232,9 +242,7 @@ export class IntersectionObserverStub { return []; } - /** - * @param {!Element} target - */ + /** @param {!Element} target */ observe(target) { if (this.inst_) { this.inst_.observe(target); @@ -245,9 +253,7 @@ export class IntersectionObserverStub { } } - /** - * @param {!Element} target - */ + /** @param {!Element} target */ unobserve(target) { if (this.inst_) { this.inst_.unobserve(target); @@ -260,11 +266,11 @@ export class IntersectionObserverStub { } /** - * @param {typeof IntersectionObserver} constr + * @param {IntersectionObserverCtor} Ctor * @private */ - upgrade_(constr) { - const inst = new constr(this.callback_, this.options_); + upgrade_(Ctor) { + const inst = new Ctor(this.callback_, this.options_); this.inst_ = inst; this.elements_.forEach((e) => inst.observe(e)); this.elements_ = null; @@ -272,7 +278,7 @@ export class IntersectionObserverStub { } /** - * @type {!Array} + * @type {!Array} */ IntersectionObserverStub[UPGRADERS] = []; From 57f2ffa82a5025a18886c8212b5f58847f79aad0 Mon Sep 17 00:00:00 2001 From: Ryan Cebulko Date: Wed, 5 May 2021 14:09:08 -0400 Subject: [PATCH 07/21] Remove @suppress {checkTypes} from promise --- src/polyfills/promise.js | 1 - 1 file changed, 1 deletion(-) diff --git a/src/polyfills/promise.js b/src/polyfills/promise.js index 8cb9e918db48..83712943d5b9 100644 --- a/src/polyfills/promise.js +++ b/src/polyfills/promise.js @@ -19,7 +19,6 @@ import Promise from 'promise-pjs'; /** * Sets the Promise polyfill if it does not exist. * @param {!Window} win - * @suppress {checkTypes} */ export function install(win) { if (!win.Promise) { From a04884421b67f0db130ba137e60a10459826cff4 Mon Sep 17 00:00:00 2001 From: Ryan Cebulko Date: Wed, 5 May 2021 14:17:52 -0400 Subject: [PATCH 08/21] Fix @this type for document.contains() polyfill --- src/polyfills/document-contains.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/polyfills/document-contains.js b/src/polyfills/document-contains.js index 546c0df9875a..5d5d2781b41c 100644 --- a/src/polyfills/document-contains.js +++ b/src/polyfills/document-contains.js @@ -20,7 +20,7 @@ * See https://developer.mozilla.org/en-US/docs/Web/API/Node/contains * @param {?Node} node * @return {boolean} - * @this {Node} + * @this {Document} */ function documentContainsPolyfill(node) { // Per spec, "contains" method is inclusionary From c6b5998a1897ce2b905250daca7496993ed7bafc Mon Sep 17 00:00:00 2001 From: Ryan Cebulko Date: Wed, 5 May 2021 14:22:52 -0400 Subject: [PATCH 09/21] Remove unneeded typecast to unknown --- src/polyfills/promise.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/polyfills/promise.js b/src/polyfills/promise.js index 83712943d5b9..37ed4f1ab6a1 100644 --- a/src/polyfills/promise.js +++ b/src/polyfills/promise.js @@ -22,7 +22,7 @@ import Promise from 'promise-pjs'; */ export function install(win) { if (!win.Promise) { - win.Promise = /** @type {?} */ (Promise); + win.Promise = Promise; // In babel the * export is an Object with a default property. // In closure compiler it is the Promise function itself. if (Promise.default) { From 2c6d43927b2b975f5654c2d1d5476d5c318e6665 Mon Sep 17 00:00:00 2001 From: Ryan Cebulko Date: Wed, 5 May 2021 15:10:19 -0400 Subject: [PATCH 10/21] Fix types and add comments in resize-observer-stub --- .../stubs/intersection-observer-stub.js | 20 +++----- src/polyfills/stubs/resize-observer-stub.js | 48 ++++++++----------- 2 files changed, 27 insertions(+), 41 deletions(-) diff --git a/src/polyfills/stubs/intersection-observer-stub.js b/src/polyfills/stubs/intersection-observer-stub.js index 6da62e9b49cf..e52cc5b734a3 100644 --- a/src/polyfills/stubs/intersection-observer-stub.js +++ b/src/polyfills/stubs/intersection-observer-stub.js @@ -25,6 +25,8 @@ /** @typedef {typeof IntersectionObserver} */ let IntersectionObserverCtor; +/** @typedef {function(IntersectionObserverCtor)} */ +let IntersectionObserverUpgrader; const UPGRADERS = '_upgraders'; const NATIVE = '_native'; @@ -129,8 +131,7 @@ export function supportsDocumentRoot(win) { export function upgradePolyfill(win, installer) { // Can't use the IntersectionObserverStub here directly since it's a separate // instance deployed in v0.js vs the polyfill extension. - const Stub = /** @type {typeof IntersectionObserverStub} */ (win - .IntersectionObserver[STUB]); + const Stub = win.IntersectionObserver[STUB]; if (Stub) { const Native = win.IntersectionObserver[NATIVE]; delete win.IntersectionObserver; @@ -144,19 +145,14 @@ export function upgradePolyfill(win, installer) { ); } + /** @type {!Array} */ const upgraders = Stub[UPGRADERS].slice(0); const microtask = Promise.resolve(); const upgrade = (upgrader) => { microtask.then(() => upgrader(Polyfill)); }; - if (upgraders.length > 0) { - /** @type {!Array} */ (upgraders).forEach(upgrade); - } - Stub[ - UPGRADERS - ] = /** @type {!Array} */ ({ - 'push': upgrade, - }); + upgraders.forEach(upgrade); + Stub[UPGRADERS] = {'push': upgrade}; } else { // Even if this is not the stub, we still may need to polyfill // `isIntersecting`. See `shouldLoadPolyfill` for more info. @@ -277,9 +273,7 @@ export class IntersectionObserverStub { } } -/** - * @type {!Array} - */ +/** @type {!Array} */ IntersectionObserverStub[UPGRADERS] = []; /** @visibleForTesting */ diff --git a/src/polyfills/stubs/resize-observer-stub.js b/src/polyfills/stubs/resize-observer-stub.js index d7631ad091d3..ed213a3a5f1e 100644 --- a/src/polyfills/stubs/resize-observer-stub.js +++ b/src/polyfills/stubs/resize-observer-stub.js @@ -23,6 +23,11 @@ * amp-resize-observer-polyfill extension. */ +/** @typedef {typeof ResizeObserver} */ +let ResizeObserverCtor; +/** @typedef {function(ResizeObserverCtor)} */ +let ResizeObserverUpgrader; + const UPGRADERS = '_upgraders'; const STUB = '_stub'; @@ -56,26 +61,21 @@ export function installStub(win) { export function upgradePolyfill(win, installer) { // Can't use the ResizeObserverStub here directly since it's a separate // instance deployed in v0.js vs the polyfill extension. - const Stub = /** @type {typeof ResizeObserverStub} */ (win.ResizeObserver[ - STUB - ]); + const Stub = win.ResizeObserver[STUB]; if (Stub) { delete win.ResizeObserver; delete win.ResizeObserverEntry; installer(); const Polyfill = win.ResizeObserver; + /** @type {!Array} */ const upgraders = Stub[UPGRADERS].slice(0); const microtask = Promise.resolve(); const upgrade = (upgrader) => { microtask.then(() => upgrader(Polyfill)); }; - if (upgraders.length > 0) { - /** @type {!Array} */ (upgraders).forEach(upgrade); - } - Stub[UPGRADERS] = /** @type {!Array} */ ({ - 'push': upgrade, - }); + upgraders.forEach(upgrade); + Stub[UPGRADERS] = {'push': upgrade}; } else { // Even if this is not the stub, we still may need to install the polyfill. // See `shouldLoadPolyfill` for more info. @@ -87,13 +87,14 @@ export function upgradePolyfill(win, installer) { * The stub for `ResizeObserver`. Implements the same interface, but * keeps the tracked elements in memory until the actual polyfill arives. * This stub is necessary because the polyfill itself is significantly bigger. + * It doesn't technically extend ResizeObserver, but this allows the stub + * to be seen as equivalent when typechecking calls expecting an ResObs. + * @extends ResizeObserver */ export class ResizeObserverStub { - /** - * @param {!ResizeObserverCallback} callback - */ + /** @param {!ResizeObserverCallback} callback */ constructor(callback) { - /** @private @const */ + /** @private @const {!ResizeObserverCallback} */ this.callback_ = callback; /** @private {?Array} */ @@ -106,9 +107,6 @@ export class ResizeObserverStub { ResizeObserverStub[UPGRADERS].push(this.upgrade_.bind(this)); } - /** - * @return {*} - */ disconnect() { if (this.inst_) { this.inst_.disconnect(); @@ -117,9 +115,7 @@ export class ResizeObserverStub { } } - /** - * @param {!Element} target - */ + /** @param {!Element} target */ observe(target) { if (this.inst_) { this.inst_.observe(target); @@ -130,9 +126,7 @@ export class ResizeObserverStub { } } - /** - * @param {!Element} target - */ + /** @param {!Element} target */ unobserve(target) { if (this.inst_) { this.inst_.unobserve(target); @@ -145,20 +139,18 @@ export class ResizeObserverStub { } /** - * @param {typeof ResizeObserver} constr + * @param {ResizeObserverCtor} Ctor * @private */ - upgrade_(constr) { - const inst = new constr(this.callback_, this.options_); + upgrade_(Ctor) { + const inst = new Ctor(this.callback_); this.inst_ = inst; this.elements_.forEach((e) => inst.observe(e)); this.elements_ = null; } } -/** - * @type {!Array} - */ +/** @type {!Array} */ ResizeObserverStub[UPGRADERS] = []; /** @visibleForTesting */ From a5956c7eccb1ba36a6c5a0d20699c6d2df1c89f8 Mon Sep 17 00:00:00 2001 From: Ryan Cebulko Date: Wed, 5 May 2021 16:05:55 -0400 Subject: [PATCH 11/21] Fix types and add comments in custom-elements --- src/polyfills/custom-elements.extern.js | 29 ++++++++ src/polyfills/custom-elements.js | 94 +++++++++---------------- src/polyfills/shame.extern.js | 15 ++-- 3 files changed, 70 insertions(+), 68 deletions(-) create mode 100644 src/polyfills/custom-elements.extern.js diff --git a/src/polyfills/custom-elements.extern.js b/src/polyfills/custom-elements.extern.js new file mode 100644 index 000000000000..affe78c49f2e --- /dev/null +++ b/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; diff --git a/src/polyfills/custom-elements.js b/src/polyfills/custom-elements.js index bd656c61c056..0d45829ccfde 100644 --- a/src/polyfills/custom-elements.js +++ b/src/polyfills/custom-elements.js @@ -14,23 +14,21 @@ * limitations under the License. */ -/** - * @typedef {{ - * promise: !Promise, - * resolve: function(), - * }} - */ -let DeferredDef; +import {Deferred} from '../core/data-structures/promise'; /** - * @typedef {!typeof HTMLElement} + * For type anotations where Element is a local variable. + * @typedef {!Element} */ -let CustomElementConstructorDef; +let ElementOrig; + +/** @typedef {!typeof HTMLElement} */ +let CustomElementCtor; /** * @typedef {{ * name: string, - * ctor: !CustomElementConstructorDef, + * ctor: !CustomElementCtor, * }} */ let CustomElementDef; @@ -102,6 +100,7 @@ function isPatched(win) { /** * Throws the error outside the current event loop. + * TODO(rcebulko): Condense with core/error#rethrowAsync * * @param {!Error} error */ @@ -121,21 +120,13 @@ class CustomElementRegistry { * @param {!Registry} registry */ constructor(win, registry) { - /** - * @const @private - */ + /** @const @private */ this.win_ = win; - /** - * @const @private - */ + /** @const @private */ this.registry_ = registry; - /** - * @type {!Object} - * @private - * @const - */ + /** @private @const @type {!Object} */ this.pendingDefines_ = Object.create(null); } @@ -143,7 +134,7 @@ class CustomElementRegistry { * Register the custom element. * * @param {string} name - * @param {!CustomElementConstructorDef} ctor + * @param {!CustomElementCtor} ctor * @param {!Object=} options */ define(name, ctor, options) { @@ -163,7 +154,7 @@ class CustomElementRegistry { * Get the constructor of the (already defined) custom element. * * @param {string} name - * @return {!CustomElementConstructorDef|undefined} + * @return {!CustomElementCtor|undefined} */ get(name) { const def = this.registry_.getByName(name); @@ -188,19 +179,13 @@ class CustomElementRegistry { } const pending = this.pendingDefines_; - const deferred = pending[name]; - if (deferred) { - return deferred.promise; + let deferred = pending[name]; + if (!deferred) { + deferred = new Deferred(); + pending[name] = deferred; } - let resolve; - const promise = new /*OK*/ Promise((res) => (resolve = res)); - pending[name] = { - promise, - resolve, - }; - - return promise; + return deferred.promise; } /** @@ -223,16 +208,10 @@ class Registry { * @param {!Window} win */ constructor(win) { - /** - * @private @const - */ + /** @private @const */ this.win_ = win; - /** - * @type {!Object} - * @private - * @const - */ + /** @private @const @type {!Object} */ this.definitions_ = Object.create(null); /** @@ -243,7 +222,7 @@ class Registry { /** * The currently upgrading element. - * @private {Element} + * @private {?Element} */ this.current_ = null; @@ -251,7 +230,7 @@ class Registry { * Once started (after the first Custom Element definition), this tracks * DOM append and removals. * - * @private {MutationObserver} + * @private {?MutationObserver} */ this.mutationObserver_ = null; @@ -273,7 +252,7 @@ class Registry { * constructor while returning this current node in the HTMLElement * class constructor (the base class of all custom elements). * - * @return {Element} + * @return {?Element} */ current() { const current = this.current_; @@ -285,7 +264,7 @@ class Registry { * Finds the custom element definition by name. * * @param {string} name - * @return {CustomElementDef|undefined} + * @return {!CustomElementDef|undefined} */ getByName(name) { const definition = this.definitions_[name]; @@ -297,8 +276,8 @@ class Registry { /** * Finds the custom element definition by constructor instance. * - * @param {CustomElementConstructorDef} ctor - * @return {CustomElementDef|undefined} + * @param {!CustomElementCtor} ctor + * @return {!CustomElementDef|undefined} */ getByConstructor(ctor) { const definitions = this.definitions_; @@ -316,7 +295,7 @@ class Registry { * name in the root document. * * @param {string} name - * @param {!CustomElementConstructorDef} ctor + * @param {!CustomElementCtor} ctor * @param {!Object|undefined} options */ define(name, ctor, options) { @@ -725,7 +704,7 @@ function polyfill(win) { const {attachShadow, createShadowRoot} = elProto; if (attachShadow) { /** - * @param {!{mode: string}} unused + * @param {{mode: string}} unused * @return {!ShadowRoot} */ elProto.attachShadow = function (unused) { @@ -739,9 +718,7 @@ function polyfill(win) { }; } if (createShadowRoot) { - /** - * @return {!ShadowRoot} - */ + /** @return {!ShadowRoot} */ elProto.createShadowRoot = function () { const shadow = createShadowRoot.apply(this, arguments); registry.observe(shadow); @@ -757,7 +734,7 @@ function polyfill(win) { * You can't use the real HTMLElement constructor, because you can't subclass * it without using native classes. So, mock its approximation using * createElement. - * @return {*} TODO(#23582): Specify return type + * @return {!ElementOrig} */ function HTMLElementPolyfill() { const {constructor} = this; @@ -825,9 +802,7 @@ function polyfill(win) { */ function wrapHTMLElement(win) { const {HTMLElement, Reflect} = win; - /** - * @return {!Element} - */ + /** @return {!Element} */ function HTMLElementWrapper() { const ctor = /** @type {function(...?):?|undefined} */ (this.constructor); @@ -846,8 +821,8 @@ function wrapHTMLElement(win) { /** * Setups up prototype inheritance * - * @param {!typeof SUPER} superClass - * @param {!typeof SUB} subClass + * @param {!SUPER} superClass + * @param {!SUB} subClass * @template SUPER * @template SUB */ @@ -882,6 +857,7 @@ function supportsUnderProto() { * old IE. * @param {!Object} obj * @param {!Object} prototype + * @suppress {suspiciousCode} due to IS_ESM inlining */ function setPrototypeOf(obj, prototype) { if (IS_ESM || Object.setPrototypeOf) { diff --git a/src/polyfills/shame.extern.js b/src/polyfills/shame.extern.js index 198c5f72ab23..5e279f59b37e 100644 --- a/src/polyfills/shame.extern.js +++ b/src/polyfills/shame.extern.js @@ -25,15 +25,12 @@ * @externs */ -/** - * These definitions are exported by the fetch and get-bounding-client-rect - * polyfill implementation files, but currently they have dependencies on - * non-core modules that aren't yet type-checked. - * - * Planned destination: These externs should be * removed when those files are - * re-included in the polyfills type-check target. - */ - +// These definitions are exported by the fetch and get-bounding-client-rect +// polyfill implementation files, but currently they have dependencies on +// non-core modules that aren't yet type-checked. +// +// Planned destination: These externs should be removed when those files are +// re-included in the polyfills type-check target. /** @type {function(!Window)} */ let install$$module$src$polyfills$fetch; /** @type {function(!Window)} */ From 9c3b753bbc05ffac0e7aa8006a31983923f566c9 Mon Sep 17 00:00:00 2001 From: Ryan Cebulko Date: Wed, 5 May 2021 16:07:21 -0400 Subject: [PATCH 12/21] Add ! --- src/polyfills/stubs/intersection-observer-stub.js | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/src/polyfills/stubs/intersection-observer-stub.js b/src/polyfills/stubs/intersection-observer-stub.js index e52cc5b734a3..5a1772599c86 100644 --- a/src/polyfills/stubs/intersection-observer-stub.js +++ b/src/polyfills/stubs/intersection-observer-stub.js @@ -145,7 +145,7 @@ export function upgradePolyfill(win, installer) { ); } - /** @type {!Array} */ + /** @type {!Array} */ const upgraders = Stub[UPGRADERS].slice(0); const microtask = Promise.resolve(); const upgrade = (upgrader) => { @@ -230,7 +230,7 @@ export class IntersectionObserverStub { } } - /** @return {!Array} */ + /** @return {!Array} */ takeRecords() { if (this.inst_) { return this.inst_.takeRecords(); @@ -262,7 +262,7 @@ export class IntersectionObserverStub { } /** - * @param {IntersectionObserverCtor} Ctor + * @param {!IntersectionObserverCtor} Ctor * @private */ upgrade_(Ctor) { From 79b6329aa1bdb497b03898dc252258bd3a6f647d Mon Sep 17 00:00:00 2001 From: Ryan Cebulko Date: Wed, 5 May 2021 16:20:00 -0400 Subject: [PATCH 13/21] Fix types in get-bounding-client-rect --- build-system/tasks/check-types.js | 5 ++-- src/polyfills/get-bounding-client-rect.js | 2 ++ src/polyfills/shame.extern.js | 36 ++++++++++++++++++----- 3 files changed, 33 insertions(+), 10 deletions(-) diff --git a/build-system/tasks/check-types.js b/build-system/tasks/check-types.js index d8eb31d7f060..6749f3477a44 100644 --- a/build-system/tasks/check-types.js +++ b/build-system/tasks/check-types.js @@ -131,9 +131,8 @@ const TYPE_CHECK_TARGETS = { 'src-polyfills': { srcGlobs: [ 'src/polyfills/**/*.js', - // Exclude fetch and get-bounding-client-rect until their dependencies are - // cleaned up/extracted to core. - '!src/polyfills/{get-bounding-client-rect,fetch}.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], diff --git a/src/polyfills/get-bounding-client-rect.js b/src/polyfills/get-bounding-client-rect.js index e26115024752..cc6d160b257a 100644 --- a/src/polyfills/get-bounding-client-rect.js +++ b/src/polyfills/get-bounding-client-rect.js @@ -34,6 +34,7 @@ let nativeClientRect; * Polyfill for Node.getBoundingClientRect API. * @this {!Element} * @return {!ClientRect|!LayoutRectDef} + * @suppress {suspiciousCode} due to IS_ESM inlining */ function getBoundingClientRect() { // eslint-disable-next-line local/no-invalid-this @@ -48,6 +49,7 @@ function getBoundingClientRect() { * Determines if this polyfill should be installed. * @param {!Window} win * @return {boolean} + * @suppress {uselessCode} due to IS_ESM inlining */ function shouldInstall(win) { if (IS_ESM) { diff --git a/src/polyfills/shame.extern.js b/src/polyfills/shame.extern.js index 5e279f59b37e..b5c0f21e1eae 100644 --- a/src/polyfills/shame.extern.js +++ b/src/polyfills/shame.extern.js @@ -25,13 +25,35 @@ * @externs */ -// These definitions are exported by the fetch and get-bounding-client-rect -// polyfill implementation files, but currently they have dependencies on -// non-core modules that aren't yet type-checked. +// This definition is exported by the fetch polyfill implementation, but +// currently has dependencies on non-core modules that aren't yet type-checked. // -// Planned destination: These externs should be removed when those files are -// re-included in the polyfills type-check target. +// Planned destination: this should be removed when fetch is re-included in the +// polyfills type-check target. /** @type {function(!Window)} */ let install$$module$src$polyfills$fetch; -/** @type {function(!Window)} */ -let install$$module$src$polyfills$get_bounding_client_rect; + +// These definitions live in non-core files but are consumed by +// get-bounding-client-rect. They should be removed as dom.js and layout-rect.js +// are moved to core. +/** + * The structure that combines position and size for an element. The exact + * interpretation of position and size depends on the use case. Replicated from + * layout-rect.js + * + * @typedef {{ + * top: number, + * bottom: number, + * left: number, + * right: number, + * width: number, + * height: number, + * x: number, + * y: number + * }} + */ +let LayoutRectDef$$module$src$layout_rect; +/** @type {function(number, number, number, number):LayoutRectDef$$module$src$layout_rect} */ +let layoutRectLtwh$$module$src$layout_rect; +/** @type {function(!Element):boolean} */ +let isConnectedNode$$module$src$dom; From cdb531d5a0da6a21222ff73ce28ec63a2baca6b8 Mon Sep 17 00:00:00 2001 From: Ryan Cebulko Date: Wed, 5 May 2021 16:33:48 -0400 Subject: [PATCH 14/21] Add more ! --- src/polyfills/stubs/resize-observer-stub.js | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/src/polyfills/stubs/resize-observer-stub.js b/src/polyfills/stubs/resize-observer-stub.js index ed213a3a5f1e..9b8110d6e27d 100644 --- a/src/polyfills/stubs/resize-observer-stub.js +++ b/src/polyfills/stubs/resize-observer-stub.js @@ -23,9 +23,9 @@ * amp-resize-observer-polyfill extension. */ -/** @typedef {typeof ResizeObserver} */ +/** @typedef {!typeof ResizeObserver} */ let ResizeObserverCtor; -/** @typedef {function(ResizeObserverCtor)} */ +/** @typedef {function(!ResizeObserverCtor)} */ let ResizeObserverUpgrader; const UPGRADERS = '_upgraders'; @@ -139,7 +139,7 @@ export class ResizeObserverStub { } /** - * @param {ResizeObserverCtor} Ctor + * @param {!ResizeObserverCtor} Ctor * @private */ upgrade_(Ctor) { @@ -150,7 +150,7 @@ export class ResizeObserverStub { } } -/** @type {!Array} */ +/** @type {!Array} */ ResizeObserverStub[UPGRADERS] = []; /** @visibleForTesting */ From 60506574c552de90fb4511028572a58e54af8c7d Mon Sep 17 00:00:00 2001 From: Ryan Cebulko Date: Wed, 5 May 2021 16:40:51 -0400 Subject: [PATCH 15/21] Lint fixes --- src/polyfills/custom-elements.js | 16 ++++++++-------- .../stubs/intersection-observer-stub.js | 19 ++++++++++--------- src/polyfills/stubs/resize-observer-stub.js | 13 +++++++------ 3 files changed, 25 insertions(+), 23 deletions(-) diff --git a/src/polyfills/custom-elements.js b/src/polyfills/custom-elements.js index 0d45829ccfde..4b98698cca2b 100644 --- a/src/polyfills/custom-elements.js +++ b/src/polyfills/custom-elements.js @@ -20,15 +20,15 @@ import {Deferred} from '../core/data-structures/promise'; * For type anotations where Element is a local variable. * @typedef {!Element} */ -let ElementOrig; +let ElementDef; /** @typedef {!typeof HTMLElement} */ -let CustomElementCtor; +let CustomElementConstructorDef; /** * @typedef {{ * name: string, - * ctor: !CustomElementCtor, + * ctor: !CustomElementConstructorDef, * }} */ let CustomElementDef; @@ -134,7 +134,7 @@ class CustomElementRegistry { * Register the custom element. * * @param {string} name - * @param {!CustomElementCtor} ctor + * @param {!CustomElementConstructorDef} ctor * @param {!Object=} options */ define(name, ctor, options) { @@ -154,7 +154,7 @@ class CustomElementRegistry { * Get the constructor of the (already defined) custom element. * * @param {string} name - * @return {!CustomElementCtor|undefined} + * @return {!CustomElementConstructorDef|undefined} */ get(name) { const def = this.registry_.getByName(name); @@ -276,7 +276,7 @@ class Registry { /** * Finds the custom element definition by constructor instance. * - * @param {!CustomElementCtor} ctor + * @param {!CustomElementConstructorDef} ctor * @return {!CustomElementDef|undefined} */ getByConstructor(ctor) { @@ -295,7 +295,7 @@ class Registry { * name in the root document. * * @param {string} name - * @param {!CustomElementCtor} ctor + * @param {!CustomElementConstructorDef} ctor * @param {!Object|undefined} options */ define(name, ctor, options) { @@ -734,7 +734,7 @@ function polyfill(win) { * You can't use the real HTMLElement constructor, because you can't subclass * it without using native classes. So, mock its approximation using * createElement. - * @return {!ElementOrig} + * @return {!ElementDef} */ function HTMLElementPolyfill() { const {constructor} = this; diff --git a/src/polyfills/stubs/intersection-observer-stub.js b/src/polyfills/stubs/intersection-observer-stub.js index 5a1772599c86..aed17fae4916 100644 --- a/src/polyfills/stubs/intersection-observer-stub.js +++ b/src/polyfills/stubs/intersection-observer-stub.js @@ -24,9 +24,9 @@ */ /** @typedef {typeof IntersectionObserver} */ -let IntersectionObserverCtor; -/** @typedef {function(IntersectionObserverCtor)} */ -let IntersectionObserverUpgrader; +let IntObsConstructorDef; +/** @typedef {function(IntObsConstructorDef)} */ +let IntObsUpgraderDef; const UPGRADERS = '_upgraders'; const NATIVE = '_native'; @@ -63,9 +63,9 @@ function isWebkit(win) { } /** - * @param {IntersectionObserverCtor} Native - * @param {IntersectionObserverCtor} Polyfill - * @return {IntersectionObserverCtor} + * @param {IntObsConstructorDef} Native + * @param {IntObsConstructorDef} Polyfill + * @return {IntObsConstructorDef} */ function getIntersectionObserverDispatcher(Native, Polyfill) { /** @@ -145,7 +145,7 @@ export function upgradePolyfill(win, installer) { ); } - /** @type {!Array} */ + /** @type {!Array} */ const upgraders = Stub[UPGRADERS].slice(0); const microtask = Promise.resolve(); const upgrade = (upgrader) => { @@ -222,6 +222,7 @@ export class IntersectionObserverStub { return [].concat(this.options_.threshold || 0); } + /** @function */ disconnect() { if (this.inst_) { this.inst_.disconnect(); @@ -262,7 +263,7 @@ export class IntersectionObserverStub { } /** - * @param {!IntersectionObserverCtor} Ctor + * @param {!IntObsConstructorDef} Ctor * @private */ upgrade_(Ctor) { @@ -273,7 +274,7 @@ export class IntersectionObserverStub { } } -/** @type {!Array} */ +/** @type {!Array} */ IntersectionObserverStub[UPGRADERS] = []; /** @visibleForTesting */ diff --git a/src/polyfills/stubs/resize-observer-stub.js b/src/polyfills/stubs/resize-observer-stub.js index 9b8110d6e27d..99e7fc0b5aa9 100644 --- a/src/polyfills/stubs/resize-observer-stub.js +++ b/src/polyfills/stubs/resize-observer-stub.js @@ -24,9 +24,9 @@ */ /** @typedef {!typeof ResizeObserver} */ -let ResizeObserverCtor; -/** @typedef {function(!ResizeObserverCtor)} */ -let ResizeObserverUpgrader; +let ResObsConstructorDef; +/** @typedef {function(!ResObsConstructorDef)} */ +let ResObsUpgraderDef; const UPGRADERS = '_upgraders'; const STUB = '_stub'; @@ -68,7 +68,7 @@ export function upgradePolyfill(win, installer) { installer(); const Polyfill = win.ResizeObserver; - /** @type {!Array} */ + /** @type {!Array} */ const upgraders = Stub[UPGRADERS].slice(0); const microtask = Promise.resolve(); const upgrade = (upgrader) => { @@ -107,6 +107,7 @@ export class ResizeObserverStub { ResizeObserverStub[UPGRADERS].push(this.upgrade_.bind(this)); } + /** @function */ disconnect() { if (this.inst_) { this.inst_.disconnect(); @@ -139,7 +140,7 @@ export class ResizeObserverStub { } /** - * @param {!ResizeObserverCtor} Ctor + * @param {!ResObsConstructorDef} Ctor * @private */ upgrade_(Ctor) { @@ -150,7 +151,7 @@ export class ResizeObserverStub { } } -/** @type {!Array} */ +/** @type {!Array} */ ResizeObserverStub[UPGRADERS] = []; /** @visibleForTesting */ From 12bd5ab66a30a5bd1d6f5dc6fb0660c387ef323b Mon Sep 17 00:00:00 2001 From: Ryan Cebulko Date: Wed, 5 May 2021 16:43:52 -0400 Subject: [PATCH 16/21] Allow custom-element.externs.js to use window --- src/.eslintrc.js | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/src/.eslintrc.js b/src/.eslintrc.js index e9e92d11580d..065a75ce1635 100644 --- a/src/.eslintrc.js +++ b/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}, }, ], From 42965225b9def2ca5a50691ffb191c533d8ffc31 Mon Sep 17 00:00:00 2001 From: Ryan Cebulko Date: Wed, 5 May 2021 17:06:45 -0400 Subject: [PATCH 17/21] Revert no-op JSDoc changes --- src/polyfills/stubs/intersection-observer-stub.js | 2 +- src/polyfills/stubs/resize-observer-stub.js | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/src/polyfills/stubs/intersection-observer-stub.js b/src/polyfills/stubs/intersection-observer-stub.js index aed17fae4916..4ffc5bcab6ad 100644 --- a/src/polyfills/stubs/intersection-observer-stub.js +++ b/src/polyfills/stubs/intersection-observer-stub.js @@ -222,7 +222,7 @@ export class IntersectionObserverStub { return [].concat(this.options_.threshold || 0); } - /** @function */ + /** @return {undefined} */ disconnect() { if (this.inst_) { this.inst_.disconnect(); diff --git a/src/polyfills/stubs/resize-observer-stub.js b/src/polyfills/stubs/resize-observer-stub.js index 99e7fc0b5aa9..85384c6b2384 100644 --- a/src/polyfills/stubs/resize-observer-stub.js +++ b/src/polyfills/stubs/resize-observer-stub.js @@ -107,7 +107,7 @@ export class ResizeObserverStub { ResizeObserverStub[UPGRADERS].push(this.upgrade_.bind(this)); } - /** @function */ + /** @return {undefined} */ disconnect() { if (this.inst_) { this.inst_.disconnect(); From 31cf506dd56142549952de0fb9bdbcba258eaad5 Mon Sep 17 00:00:00 2001 From: Ryan Cebulko Date: Wed, 5 May 2021 17:15:40 -0400 Subject: [PATCH 18/21] Remove connected callback externs and typecast instead --- src/polyfills/custom-elements.extern.js | 9 --------- src/polyfills/custom-elements.js | 4 +++- 2 files changed, 3 insertions(+), 10 deletions(-) diff --git a/src/polyfills/custom-elements.extern.js b/src/polyfills/custom-elements.extern.js index affe78c49f2e..d05302e616fb 100644 --- a/src/polyfills/custom-elements.extern.js +++ b/src/polyfills/custom-elements.extern.js @@ -18,12 +18,3 @@ // 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; diff --git a/src/polyfills/custom-elements.js b/src/polyfills/custom-elements.js index 4b98698cca2b..db95ceeb8b1e 100644 --- a/src/polyfills/custom-elements.js +++ b/src/polyfills/custom-elements.js @@ -424,7 +424,8 @@ class Registry { if (!def) { return; } - this.upgradeSelf_(/** @type {!Element} */ (node), def); + node = /** @type {!HTMLElement} */ (node); + this.upgradeSelf_(node, def); // TODO(jridgewell): It may be appropriate to adoptCallback, if the node // used to be in another doc. // TODO(jridgewell): I should be calling the definitions connectedCallback @@ -446,6 +447,7 @@ class Registry { disconnectedCallback_(node) { // TODO(jridgewell): I should be calling the definitions connectedCallback // with node as the context. + node = /** @type {!HTMLElement} */ (node); if (node.disconnectedCallback) { try { node.disconnectedCallback(); From 228d1cf47c02459808a71e7eeefb160008ae518d Mon Sep 17 00:00:00 2001 From: Ryan Cebulko Date: Wed, 5 May 2021 17:32:57 -0400 Subject: [PATCH 19/21] Clean up typedefs for observer stubs --- .../stubs/intersection-observer-stub.js | 21 +++++++++---------- src/polyfills/stubs/resize-observer-stub.js | 8 +++---- 2 files changed, 14 insertions(+), 15 deletions(-) diff --git a/src/polyfills/stubs/intersection-observer-stub.js b/src/polyfills/stubs/intersection-observer-stub.js index 4ffc5bcab6ad..5513c28c2fc8 100644 --- a/src/polyfills/stubs/intersection-observer-stub.js +++ b/src/polyfills/stubs/intersection-observer-stub.js @@ -23,10 +23,8 @@ * amp-intersection-observer-polyfill extension. */ -/** @typedef {typeof IntersectionObserver} */ -let IntObsConstructorDef; -/** @typedef {function(IntObsConstructorDef)} */ -let IntObsUpgraderDef; +/** @typedef {function(!typeof IntersectionObserver)} */ +let InObUpgraderDef; const UPGRADERS = '_upgraders'; const NATIVE = '_native'; @@ -63,9 +61,9 @@ function isWebkit(win) { } /** - * @param {IntObsConstructorDef} Native - * @param {IntObsConstructorDef} Polyfill - * @return {IntObsConstructorDef} + * @param {!typeof IntersectionObserver} Native + * @param {!typeof IntersectionObserver} Polyfill + * @return {!typeof IntersectionObserver} */ function getIntersectionObserverDispatcher(Native, Polyfill) { /** @@ -145,7 +143,7 @@ export function upgradePolyfill(win, installer) { ); } - /** @type {!Array} */ + /** @type {!Array} */ const upgraders = Stub[UPGRADERS].slice(0); const microtask = Promise.resolve(); const upgrade = (upgrader) => { @@ -166,7 +164,8 @@ export function upgradePolyfill(win, installer) { * This stub is necessary because the polyfill itself is significantly bigger. * * It doesn't technically extend IntersectionObserver, but this allows the stub - * to be seen as equivalent when typechecking calls expecting an IntObs. + * to be seen as equivalent when typechecking calls expecting an + * IntersectionObserver. * @extends IntersectionObserver */ export class IntersectionObserverStub { @@ -263,7 +262,7 @@ export class IntersectionObserverStub { } /** - * @param {!IntObsConstructorDef} Ctor + * @param {!typeof IntersectionObserver} Ctor * @private */ upgrade_(Ctor) { @@ -274,7 +273,7 @@ export class IntersectionObserverStub { } } -/** @type {!Array} */ +/** @type {!Array} */ IntersectionObserverStub[UPGRADERS] = []; /** @visibleForTesting */ diff --git a/src/polyfills/stubs/resize-observer-stub.js b/src/polyfills/stubs/resize-observer-stub.js index 85384c6b2384..80d28cf0fbc2 100644 --- a/src/polyfills/stubs/resize-observer-stub.js +++ b/src/polyfills/stubs/resize-observer-stub.js @@ -24,8 +24,8 @@ */ /** @typedef {!typeof ResizeObserver} */ -let ResObsConstructorDef; -/** @typedef {function(!ResObsConstructorDef)} */ +let!typeof ResizeObserver; +/** @typedef {function(!typeof ResizeObserver)} */ let ResObsUpgraderDef; const UPGRADERS = '_upgraders'; @@ -88,7 +88,7 @@ export function upgradePolyfill(win, installer) { * keeps the tracked elements in memory until the actual polyfill arives. * This stub is necessary because the polyfill itself is significantly bigger. * It doesn't technically extend ResizeObserver, but this allows the stub - * to be seen as equivalent when typechecking calls expecting an ResObs. + * to be seen as equivalent when typechecking calls expecting a ResizeObserver. * @extends ResizeObserver */ export class ResizeObserverStub { @@ -140,7 +140,7 @@ export class ResizeObserverStub { } /** - * @param {!ResObsConstructorDef} Ctor + * @param {!typeof ResizeObserver} Ctor * @private */ upgrade_(Ctor) { From 7bb8c269da4eef75701ed19fd38351bf476900eb Mon Sep 17 00:00:00 2001 From: Ryan Cebulko Date: Wed, 5 May 2021 17:57:04 -0400 Subject: [PATCH 20/21] Fix typo --- src/polyfills/stubs/resize-observer-stub.js | 2 -- 1 file changed, 2 deletions(-) diff --git a/src/polyfills/stubs/resize-observer-stub.js b/src/polyfills/stubs/resize-observer-stub.js index 80d28cf0fbc2..bd7321910bee 100644 --- a/src/polyfills/stubs/resize-observer-stub.js +++ b/src/polyfills/stubs/resize-observer-stub.js @@ -23,8 +23,6 @@ * amp-resize-observer-polyfill extension. */ -/** @typedef {!typeof ResizeObserver} */ -let!typeof ResizeObserver; /** @typedef {function(!typeof ResizeObserver)} */ let ResObsUpgraderDef; From 0a1a538d034eb06db68ce6513ecc8b0f136f0b26 Mon Sep 17 00:00:00 2001 From: Ryan Cebulko Date: Wed, 5 May 2021 18:44:13 -0400 Subject: [PATCH 21/21] Dedupe typedef --- src/polyfills/custom-elements.js | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/polyfills/custom-elements.js b/src/polyfills/custom-elements.js index db95ceeb8b1e..bc1389709fc2 100644 --- a/src/polyfills/custom-elements.js +++ b/src/polyfills/custom-elements.js @@ -20,7 +20,7 @@ import {Deferred} from '../core/data-structures/promise'; * For type anotations where Element is a local variable. * @typedef {!Element} */ -let ElementDef; +let ElementOrigDef; /** @typedef {!typeof HTMLElement} */ let CustomElementConstructorDef; @@ -736,7 +736,7 @@ function polyfill(win) { * You can't use the real HTMLElement constructor, because you can't subclass * it without using native classes. So, mock its approximation using * createElement. - * @return {!ElementDef} + * @return {!ElementOrigDef} */ function HTMLElementPolyfill() { const {constructor} = this;