Skip to content

Commit

Permalink
♻️ Enable type-checking src/polyfills in CI (ampproject#34239)
Browse files Browse the repository at this point in the history
* Clean up directory globs

* Move src/core srcs+externs to const arrays

* Exclude fetch/get-bounding-client-rect for now

* Provide extern for promise-pjs polyfill

* Fix types and add comments in abort-controller

* Fix types and add comments in intersection-observer-stub

* Remove @Suppress {checkTypes} from promise

* Fix @this type for document.contains() polyfill

* Remove unneeded typecast to unknown

* Fix types and add comments in resize-observer-stub

* Fix types and add comments in custom-elements

* Add !

* Fix types in get-bounding-client-rect

* Add more !

* Lint fixes

* Allow custom-element.externs.js to use window

* Revert no-op JSDoc changes

* Remove connected callback externs and typecast instead

* Clean up typedefs for observer stubs

* Fix typo

* Dedupe typedef
  • Loading branch information
rcebulko authored and rochapablo committed Aug 30, 2021
1 parent 7816f58 commit d69857a
Show file tree
Hide file tree
Showing 12 changed files with 248 additions and 169 deletions.
49 changes: 29 additions & 20 deletions build-system/tasks/check-types.js
Original file line number Diff line number Diff line change
Expand Up @@ -70,7 +70,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
Expand Down Expand Up @@ -98,55 +106,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',
},

Expand All @@ -157,7 +166,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],
},

/*
Expand Down
5 changes: 4 additions & 1 deletion src/.eslintrc.js
Original file line number Diff line number Diff line change
Expand Up @@ -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},
},
],
Expand Down
27 changes: 11 additions & 16 deletions src/polyfills/abort-controller.js
Original file line number Diff line number Diff line change
Expand Up @@ -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() {
Expand All @@ -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) {
Expand Down
20 changes: 20 additions & 0 deletions src/polyfills/custom-elements.extern.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,20 @@
/**
* 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;
Loading

0 comments on commit d69857a

Please sign in to comment.