Skip to content

Commit

Permalink
♻️ Migrate most of #core to pass TS typechecking (#37141)
Browse files Browse the repository at this point in the history
* Pass tsc for #core/mode

* Pass tsc for #core/constants

* Pass tsc for #core/types

* Pass tsc for #core/data-structures

* Pass tsc for #core/error

* Pass tsc for #core/document

* Pass tsc for #core/window

* Rebase/merge fix

* Fix stragglers

Pass tsc for #core/dom

* Pass tsc for #core/*.js

* Remove all extern files in core

* Remove ! from annotations (used by closure, implied by TS)

* Lint fixes

* Add .d.ts to lint globs

* Lint fixes

* Remove outdated AsyncInput interface decl

* Remove .d.ts from lint globs

* `readonly` syntax instead of ReadonlyArray<>

* Disable closure type-checking during presubmit.

* Lint fixes

* Lint fixes

* Enable strict null checking and fix...everything

* Rename tuple typedefs for uniqueness

* Fix type definition files

* Fix typedef location

* Fix typos and type decls ofr AMP_REPORT_ERROR

* Add missing fullscreen externs

* More fullscreen typefills

* Remove createdCallback from html-element.d.ts

* Remove comment

* Revert build-system/global.d.ts lint change
  • Loading branch information
rcebulko authored Dec 9, 2021
1 parent 47f5ee2 commit 58a1c29
Show file tree
Hide file tree
Showing 100 changed files with 1,076 additions and 938 deletions.
9 changes: 1 addition & 8 deletions build-system/tasks/check-types.js
Original file line number Diff line number Diff line change
Expand Up @@ -46,6 +46,7 @@ const getExtensionSrcPaths = () =>
const TSC_TYPECHECK_TARGETS = {
'compiler': 'src/compiler',
'carousel': 'extensions/amp-carousel/0.1',
'core': 'src/core',
};

/**
Expand Down Expand Up @@ -73,18 +74,10 @@ const CLOSURE_TYPE_CHECK_TARGETS = {
srcGlobs: ['src/amp-story-player/**/*.js'],
warningLevel: 'QUIET',
},
'src-core': CORE_SRCS_GLOBS,
'src-experiments': ['src/experiments/**/*.js', ...CORE_SRCS_GLOBS],
'src-inabox': {
srcGlobs: ['src/inabox/**/*.js'],
warningLevel: 'QUIET',
},
'src-polyfills': [
'src/polyfills/**/*.js',
// Exclude fetch its dependencies are cleaned up/extracted to core.
'!src/polyfills/fetch.js',
...CORE_SRCS_GLOBS,
],
'src-preact': {
srcGlobs: ['src/preact/**/*.js', ...CORE_SRCS_GLOBS],
warningLevel: 'QUIET',
Expand Down
1 change: 0 additions & 1 deletion build-system/test-configs/forbidden-terms.js
Original file line number Diff line number Diff line change
Expand Up @@ -632,7 +632,6 @@ const forbiddenTermsGlobal = {
'build-system/tasks/default-task.js',
'build-system/tasks/dist.js',
'src/config.js',
'src/core/window/window.extern.js',
'src/experiments/index.js',
'src/experiments/shame.extern.js',
'src/mode.js',
Expand Down
2 changes: 1 addition & 1 deletion src/compiler/types.d.ts
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,7 @@ import {TreeProto, NodeProto} from '@ampproject/bento-compiler';
* The return value is set to *, so that we may optionally return any DOM nodes
* created during a client-side render. These nodes are often needed for ivars.
*/
export type BuildDom = (Element) => void;
export type BuildDom = (el: Element) => void;

/**
* Contains component versioning data via a map from tagName --> version.
Expand Down
6 changes: 3 additions & 3 deletions src/core/3p-frame-messaging.js
Original file line number Diff line number Diff line change
Expand Up @@ -55,11 +55,11 @@ export const MessageType_Enum = {

/**
* Listens for the specified event on the element.
* @param {!EventTarget} element
* @param {EventTarget} element
* @param {string} eventType
* @param {function(!Event)} listener
* @param {function(Event):void} listener
* @param {Object=} opt_evtListenerOpts
* @return {!UnlistenDef}
* @return {UnlistenCallback}
*/
export function listen(element, eventType, listener, opt_evtListenerOpts) {
return internalListenImplementation(
Expand Down
9 changes: 9 additions & 0 deletions src/core/amp-config.d.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,9 @@
export {};

declare global {
// AMP configuration and runtime settings structure.
interface AmpConfigDef {
test?: boolean;
localDev?: boolean;
}
}
7 changes: 7 additions & 0 deletions src/core/assert.shame.d.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,7 @@
declare module '#core/assert' {
function devAssert<T>(subject: T | null | undefined, msg?: string, ...any): T;
function userAssert<T>(subject: T | null | undefined, msg?: string, ...any): T;
function devAssertElement<T>(subject: T, msg?: string, ...any): HTMLElement;
function devAssertString<T>(subject: T, msg?: string, ...any): string;
function devAssertNumber<T>(subject: T, msg?: string, ...any): number;
}
1 change: 1 addition & 0 deletions src/core/assert/index.js
Original file line number Diff line number Diff line change
@@ -1,2 +1,3 @@
// @ts-nocheck
export * from './dev';
export * from './user';
6 changes: 4 additions & 2 deletions src/core/constants/action-constants.js
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,9 @@ import {devAssert} from '#core/assert';
* The action service delegates parsing of object literals to the corresponding
* extension (in the example above, amp-bind).
*
* @see ./service/action-impl.ActionInfoDef
* See ./service/action-impl.ActionInfoDef
* TODO(rcebulko): Revert to @see once type is available
*
* @const {string}
*/
export const RAW_OBJECT_ARGS_KEY = '__AMP_OBJECT_STRING__';
Expand Down Expand Up @@ -55,7 +57,7 @@ export const ActionTrust_Enum = {
};

/**
* @param {!ActionTrust_Enum} actionTrust
* @param {ActionTrust_Enum} actionTrust
* @return {string}
*/
export function actionTrustToString(actionTrust) {
Expand Down
17 changes: 17 additions & 0 deletions src/core/constants/async-input.d.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,17 @@
/**
* Interface for all AMP Async Input Elements.
* enforces the overridable function, getValue().
* Async Input should be implemented
* by components like AMP form, to async request
* a value from a component, and then be used for
* some other action. For examples, this can be used
* by reCAPTCHA to request tokens for the form.
*
* NOTE: Elements that implemented AsyncInput must
* Also add and follow the other exported constants.
* See amp-recaptcha-input as an example.
*/
export interface AsyncInput {
// Called to get the asynchronous value of an AsyncInput field.
getValue: () => Promise<string>;
}
24 changes: 0 additions & 24 deletions src/core/constants/async-input.js
Original file line number Diff line number Diff line change
@@ -1,27 +1,3 @@
/**
* Interface for all AMP Async Input Elements.
* enforces the overridable function, getValue().
* Async Input should be implemented
* by components like AMP form, to async request
* a value from a component, and then be used for
* some other action. For examples, this can be used
* by reCAPTCHA to request tokens for the form.
*
* NOTE: Elements that implemented AsyncInput must
* Also add and follow the other exported constants.
* See amp-recaptcha-input as an example.
*
* @interface
*/
export class AsyncInput {
/**
* Called to get the asynchronous value of an
* AsyncInput field.
* @return {!Promise<string>}
*/
getValue() {}
}

/**
* Attributes
*
Expand Down
10 changes: 5 additions & 5 deletions src/core/constants/loading-instructions.js
Original file line number Diff line number Diff line change
Expand Up @@ -31,15 +31,15 @@ export const Loading_Enum = {
UNLOAD: 'unload',
};

/** @const {!Array<!Loading_Enum>} */
/** @const {Array<Loading_Enum>} */
const ORDER = [
Loading_Enum.AUTO,
Loading_Enum.LAZY,
Loading_Enum.EAGER,
Loading_Enum.UNLOAD,
];

/** @const {!Object<string, number>} */
/** @const {Object<string, number>} */
const MAP = {
[Loading_Enum.AUTO]: 0,
[Loading_Enum.LAZY]: 1,
Expand All @@ -51,9 +51,9 @@ const MAP = {
* Returns the loading instruction with a higher priority. The priority
* order is auto -> lazy -> eager -> unload.
*
* @param {!Loading_Enum|string} v1
* @param {!Loading_Enum|string} v2
* @return {!Loading_Enum}
* @param {Loading_Enum|string} v1
* @param {Loading_Enum|string} v2
* @return {Loading_Enum}
*/
export function reducer(v1, v2) {
const ordinal1 = MAP[v1] || 0;
Expand Down
28 changes: 14 additions & 14 deletions src/core/data-structures/curve.js
Original file line number Diff line number Diff line change
Expand Up @@ -20,7 +20,7 @@ export let CurveDef;
* @param {number} y1 Y coordinate of the first control point.
* @param {number} x2 X coordinate of the second control point.
* @param {number} y2 Y coordinate of the second control point.
* @return {!CurveDef}
* @return {CurveDef}
*/
export function bezierCurve(x1, y1, x2, y2) {
return (xVal) =>
Expand Down Expand Up @@ -198,57 +198,57 @@ class Bezier {
/**
* A collection of common curves.
* See https://developer.mozilla.org/en-US/docs/Web/CSS/timing-function
* @enum {!CurveDef}
* @enum {CurveDef}
*/
export const Curves_Enum = {
/**
* linear
* @param {!NormTimeDef} xVal
* @return {!NormTimeDef}
* @param {NormTimeDef} xVal
* @return {NormTimeDef}
*/
LINEAR(xVal) {
return xVal;
},

/**
* ease
* @param {!NormTimeDef} xVal
* @return {!NormTimeDef}
* @param {NormTimeDef} xVal
* @return {NormTimeDef}
*/
EASE(xVal) {
return Bezier.solveYValueFromXValue(xVal, 0, 0, 0.25, 0.1, 0.25, 1.0, 1, 1);
},

/**
* ease-in: slow out, fast in
* @param {!NormTimeDef} xVal
* @return {!NormTimeDef}
* @param {NormTimeDef} xVal
* @return {NormTimeDef}
*/
EASE_IN(xVal) {
return Bezier.solveYValueFromXValue(xVal, 0, 0, 0.42, 0.0, 1.0, 1.0, 1, 1);
},

/**
* ease-out: fast out, slow in
* @param {!NormTimeDef} xVal
* @return {!NormTimeDef}
* @param {NormTimeDef} xVal
* @return {NormTimeDef}
*/
EASE_OUT(xVal) {
return Bezier.solveYValueFromXValue(xVal, 0, 0, 0.0, 0.0, 0.58, 1.0, 1, 1);
},

/**
* ease-in-out
* @param {!NormTimeDef} xVal
* @return {!NormTimeDef}
* @param {NormTimeDef} xVal
* @return {NormTimeDef}
*/
EASE_IN_OUT(xVal) {
return Bezier.solveYValueFromXValue(xVal, 0, 0, 0.42, 0.0, 0.58, 1.0, 1, 1);
},
};

/**
* @const {!Object<string, !CurveDef>}
* @const {Object<string, CurveDef>}
*/
const NAME_MAP = {
'linear': Curves_Enum.LINEAR,
Expand Down Expand Up @@ -287,5 +287,5 @@ export function getCurve(curve) {
}
return NAME_MAP[curve];
}
return /** @type {!CurveDef} */ (curve);
return /** @type {CurveDef} */ (curve);
}
10 changes: 5 additions & 5 deletions src/core/data-structures/dom-based-weakref.js
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,7 @@
*/
export class DomBasedWeakRef {
/**
* @param {!Window} win
* @param {Window} win
* @param {string} id
* @package
*/
Expand All @@ -21,9 +21,9 @@ export class DomBasedWeakRef {
/**
* Returns a WeakRef. Uses this implementation if the real WeakRef class
* is not available.
* @param {!Window} win
* @param {!Element} element
* @return {!WeakRef<!Element>|!DomBasedWeakRef<!Element>}
* @param {Window} win
* @param {Element} element
* @return {WeakRef<Element>|DomBasedWeakRef}
*/
static make(win, element) {
if (win.WeakRef) {
Expand All @@ -36,7 +36,7 @@ export class DomBasedWeakRef {
return new DomBasedWeakRef(win, element.id);
}

/** @return {!Element|undefined} */
/** @return {Element|undefined} */
deref() {
return this.win.document.getElementById(this.id_) || undefined;
}
Expand Down
4 changes: 2 additions & 2 deletions src/core/data-structures/finite-state-machine.js
Original file line number Diff line number Diff line change
Expand Up @@ -18,7 +18,7 @@ export class FiniteStateMachine {
/**
* Callbacks that are invoked when transitioning from an old state
* to the new.
* @private {Object<string, function()>}
* @private {Object<string, function():void>}
*/
this.transitions_ = map();
}
Expand All @@ -28,7 +28,7 @@ export class FiniteStateMachine {
* transitions to the newState.
* @param {STATE} oldState
* @param {STATE} newState
* @param {function()} callback
* @param {function():void} callback
*/
addTransition(oldState, newState, callback) {
const transition = this.statesToTransition_(oldState, newState);
Expand Down
4 changes: 2 additions & 2 deletions src/core/data-structures/lru-cache.js
Original file line number Diff line number Diff line change
Expand Up @@ -20,7 +20,7 @@ export class LruCache {
*/
this.access_ = 0;

/** @private {!Object<(number|string), {payload: T, access: number}>} */
/** @private {Object<(number|string), {payload: T, access: number}>} */
this.cache_ = map();
}

Expand All @@ -36,7 +36,7 @@ export class LruCache {

/**
* @param {number|string} key
* @return {T} The cached payload.
* @return {T|undefined} The cached payload.
*/
get(key) {
const cacheable = this.cache_[key];
Expand Down
8 changes: 4 additions & 4 deletions src/core/data-structures/observable.js
Original file line number Diff line number Diff line change
Expand Up @@ -10,14 +10,14 @@ export class Observable {
* Creates an instance of Observable.
*/
constructor() {
/** @type {?Array<function(TYPE)>} */
/** @type {?Array<function(TYPE=):void>} */
this.handlers_ = null;
}

/**
* Adds the observer to this instance.
* @param {function(TYPE)} handler Observer's handler.
* @return {!UnlistenDef}
* @param {function(TYPE=):void} handler Observer's handler.
* @return {UnlistenCallback}
*/
add(handler) {
if (!this.handlers_) {
Expand All @@ -31,7 +31,7 @@ export class Observable {

/**
* Removes the observer from this instance.
* @param {function(TYPE)} handler Observer's instance.
* @param {function(TYPE=):void} handler Observer's instance.
*/
remove(handler) {
if (!this.handlers_) {
Expand Down
6 changes: 3 additions & 3 deletions src/core/data-structures/priority-queue.js
Original file line number Diff line number Diff line change
Expand Up @@ -13,7 +13,7 @@ export class PriorityQueue {

/**
* Returns the max priority item without dequeueing it.
* @return {T}
* @return {?T}
*/
peek() {
const l = this.length;
Expand Down Expand Up @@ -67,7 +67,7 @@ export class PriorityQueue {
}

/**
* @param {function(T)} callback
* @param {function(T):any} callback
*/
forEach(callback) {
let index = this.length;
Expand All @@ -79,7 +79,7 @@ export class PriorityQueue {
/**
* Dequeues the max priority item.
* Items with the same priority are dequeued in FIFO order.
* @return {T}
* @return {?T}
*/
dequeue() {
if (!this.length) {
Expand Down
Loading

0 comments on commit 58a1c29

Please sign in to comment.