Skip to content

Commit

Permalink
♻️ Enable passing type-checking on src/context (#34387)
Browse files Browse the repository at this point in the history 
* enable src-context type-check target

* Fix type errors in src/context

* export ContextPropDef

* for...of in scan

* clean up scheduler types

* Update ContextPropDef in subscriber

* update types in index

* update types in index

* update types in contextprops

* update types and for...of in node

* Drastically narrow types in values

* Add DEP template type to contextpropdef

* type-narrow subscriber

* Clean up scheduler type decl

* use templates in scan

* Update index prop and setter types

* Allow subscriber ID template type

* Add deps template arg for contextprops

* Add SID for return type

* Assert non-null subscriber IDs

* Code review tweak

* Remove template type from static function

* forEach -> for..of

* Revert forEach changes to Maps
  • Loading branch information
@rcebulko
rcebulko committed May 17, 2021
1 parent c43b21b commit 8cb5da6
Show file tree
Hide file tree
Showing 11 changed files with 252 additions and 188 deletions.
4 changes: 2 additions & 2 deletions build-system/tasks/check-types.js
View file
Expand Up @@ -110,8 +110,8 @@ const TYPE_CHECK_TARGETS = {
warningLevel: 'QUIET',
},
'src-context': {
srcGlobs: ['src/context/**/*.js'],
warningLevel: 'QUIET',
srcGlobs: ['src/context/**/*.js', ...CORE_SRCS_GLOBS],
externGlobs: ['src/context/**/*.extern.js', ...CORE_EXTERNS_GLOBS],
},
'src-core': {
srcGlobs: CORE_SRCS_GLOBS,
Expand Down
9 changes: 6 additions & 3 deletions src/context/contextprops.js
View file
Expand Up @@ -17,6 +17,9 @@
import {Loading, reducer as loadingReducer} from '../core/loading-instructions';
import {contextProp} from './prop';

// typedef imports
import {ContextPropDef} from './prop.type';

/**
* Defines whether a DOM subtree can be currently seen by the user. A subtree
* can be not renderable due `display: none`, or `hidden` attribute, unslotted
Expand All @@ -25,7 +28,7 @@ import {contextProp} from './prop';
*
* Default is `true`.
*
* @const {!ContextProp<boolean>}
* @const {!ContextPropDef<boolean>}
*/
const CanRender = contextProp('CanRender', {
defaultValue: true,
Expand All @@ -42,7 +45,7 @@ const CanRender = contextProp('CanRender', {
*
* Default is `true`.
*
* @const {!ContextProp<boolean>}
* @const {!ContextPropDef<boolean, boolean>}
*/
const CanPlay = contextProp('CanPlay', {
defaultValue: true,
Expand All @@ -59,7 +62,7 @@ const CanPlay = contextProp('CanPlay', {
*
* Default is "auto".
*
* @const {!ContextProp<!Loading>}
* @const {!ContextPropDef<!Loading, boolean>}
*/
const LoadingProp = contextProp('Loading', {
defaultValue: Loading.AUTO,
Expand Down
22 changes: 13 additions & 9 deletions src/context/index.js
View file
Expand Up @@ -15,10 +15,12 @@
*/

import {ContextNode} from './node';

export {contextProp} from './prop';
export {subscribe, unsubscribe} from './subscriber';

// typedef imports
import {ContextPropDef} from './prop.type';

/**
* Direct slot assignment. Works the same way as shadow slots, but does not
* require a shadow root. Automatically starts the discovery phase for the
Expand Down Expand Up @@ -97,8 +99,8 @@ export function rediscoverChildren(node) {
* All dependent properties are also recalculated.
*
* @param {!Node} node The target node.
* @param {!ContextProp<T>} prop
* @param {*} setter
* @param {!ContextPropDef<T>} prop
* @param {function(T)} setter
* @param {T} value
* @template T
*/
Expand All @@ -111,8 +113,9 @@ export function setProp(node, prop, setter, value) {
* See `setProp()` for more info.
*
* @param {!Node} node The target node.
* @param {!ContextProp} prop
* @param {*} setter
* @param {!ContextPropDef<T>} prop
* @param {function(T)} setter
* @template T
*/
export function removeProp(node, prop, setter) {
ContextNode.get(node).values.remove(prop, setter);
Expand All @@ -131,8 +134,8 @@ export function addGroup(node, name, match, weight = 0) {
/**
* @param {!Node} node
* @param {string} groupName
* @param {!ContextProp<T>} prop
* @param {*} setter
* @param {!ContextPropDef<T>} prop
* @param {function(T)} setter
* @param {T} value
* @template T
*/
Expand All @@ -143,8 +146,9 @@ export function setGroupProp(node, groupName, prop, setter, value) {
/**
* @param {!Node} node
* @param {string} groupName
* @param {!ContextProp} prop
* @param {*} setter
* @param {!ContextPropDef<T>} prop
* @param {function(T)} setter
* @template T
*/
export function removeGroupProp(node, groupName, prop, setter) {
ContextNode.get(node).group(groupName).values.remove(prop, setter);
Expand Down
90 changes: 37 additions & 53 deletions src/context/node.js
View file
Expand Up @@ -15,11 +15,14 @@
*/

import {Values} from './values';
import {devAssert} from '../core/assert';
import {devAssert, devAssertElement} from '../core/assert';
import {getMode} from '../mode';
import {pushIfNotExist, removeItem} from '../core/types/array';
import {throttleTail} from './scheduler';

// typedef imports
import {ContextPropDef} from './prop.type';

// Properties set on the DOM nodes to track the context state.
const NODE_PROP = '__AMP_NODE';
const ASSIGNED_SLOT_PROP = '__AMP_ASSIGNED_SLOT';
Expand Down Expand Up @@ -50,6 +53,7 @@ let GroupDef;
* are auto-discovered or prompted.
*
* @package
* @template SID subscriber ID type(s)
*/
export class ContextNode {
/**
Expand Down Expand Up @@ -105,16 +109,18 @@ export class ContextNode {
return /** @type {!ContextNode} */ (n[NODE_PROP]);
}
const {nodeType} = n;
if (nodeType == DOCUMENT_NODE || nodeType == FRAGMENT_NODE) {
if (
// A context node is always created for a root. Due to this, a
// non-root element is always at least attached to a root. This
// allows for quick discovery and reattachment when new information
// becomes available.
return ContextNode.get(n);
}
if (nodeType == ELEMENT_NODE && n.tagName.startsWith(AMP_PREFIX)) {
nodeType == DOCUMENT_NODE ||
nodeType == FRAGMENT_NODE ||
// An AMP node will always have a context node backing it at some
// point.
(nodeType == ELEMENT_NODE &&
devAssertElement(n).tagName.startsWith(AMP_PREFIX))
) {
return ContextNode.get(n);
}
}
Expand Down Expand Up @@ -173,10 +179,7 @@ export class ContextNode {
*/
static rediscoverChildren(node) {
const contextNode = /** @type {!ContextNode|undefined} */ (node[NODE_PROP]);
const children = contextNode && contextNode.children;
if (children) {
children.forEach(discoverContextNode);
}
contextNode?.children?.forEach(discoverContextNode);
}

/**
Expand Down Expand Up @@ -233,7 +236,7 @@ export class ContextNode {
/** @package {!Values} */
this.values = new Values(this);

/** @private {?Map<*, !./subscriber.Subscriber>} */
/** @private {?Map<!SID, !./subscriber.Subscriber>} */
this.subscribers_ = null;

/** @private {boolean} */
Expand All @@ -250,14 +253,9 @@ export class ContextNode {
node.addEventListener('slotchange', (e) => {
const slot = /** @type {!HTMLSlotElement} */ (e.target);
// Rediscover newly assigned nodes.
const assignedNodes = slot.assignedNodes();
assignedNodes.forEach(discoverContained);
slot.assignedNodes().forEach(discoverContained);
// Rediscover unassigned nodes.
const closest = ContextNode.closest(slot);
const closestChildren = closest && closest.children;
if (closestChildren) {
closestChildren.forEach(discoverContextNode);
}
ContextNode.closest(slot)?.children?.forEach(discoverContextNode);
});
}

Expand Down Expand Up @@ -293,10 +291,9 @@ export class ContextNode {
* @param {!ContextNode|!Node|null} parent
*/
setParent(parent) {
const parentContext =
parent && parent.nodeType
? ContextNode.get(/** @type {!Node} */ (parent))
: /** @type {?ContextNode} */ (parent);
const parentContext = parent?.nodeType
? ContextNode.get(/** @type {!Node} */ (parent))
: /** @type {?ContextNode} */ (parent);
this.updateTree_(parentContext, /* parentOverridden */ parent != null);
}

Expand All @@ -308,7 +305,7 @@ export class ContextNode {
*/
setIsRoot(isRoot) {
this.isRoot = isRoot;
const newRoot = isRoot ? this : this.parent ? this.parent.root : null;
const newRoot = isRoot ? this : this.parent?.root ?? null;
this.updateRoot(newRoot);
}

Expand All @@ -327,17 +324,10 @@ export class ContextNode {
this.values.rootUpdated();

// Make sure the tree changes have been reflected for subscribers.
const subscribers = this.subscribers_;
if (subscribers) {
subscribers.forEach((comp) => {
comp.rootUpdated();
});
}
this.subscribers_?.forEach((comp) => comp.rootUpdated());

// Propagate the root to the subtree.
if (this.children) {
this.children.forEach((child) => child.updateRoot(root));
}
this.children?.forEach((child) => child.updateRoot(root));
}
}

Expand All @@ -353,9 +343,7 @@ export class ContextNode {
const cn = new ContextNode(node, name);
groups.set(name, {cn, match, weight});
cn.setParent(this);
if (children) {
children.forEach(discoverContextNode);
}
children?.forEach(discoverContextNode);
return cn;
}

Expand All @@ -364,9 +352,7 @@ export class ContextNode {
* @return {?ContextNode}
*/
group(name) {
const {groups} = this;
const group = groups && groups.get(name);
return (group && group.cn) || null;
return this.groups?.get(name)?.cn || null;
}

/**
Expand Down Expand Up @@ -395,28 +381,28 @@ export class ContextNode {
* yet exist, it will be created using the specified factory. The use
* of factory is important to reduce bundling costs for context node.
*
* @param {*} id
* @param {function(new:./subscriber.Subscriber, function(...?), !Array<!ContextProp>):void} constr
* @param {!SID} id
* @param {typeof ./subscriber.Subscriber} Ctor
* @param {!Function} func
* @param {!Array<!ContextProp>} deps
* @param {!Array<!ContextPropDef>} deps
*/
subscribe(id, constr, func, deps) {
subscribe(id, Ctor, func, deps) {
const subscribers = this.subscribers_ || (this.subscribers_ = new Map());
let subscriber = subscribers.get(id);
if (!subscriber) {
subscriber = new constr(this, func, deps);
subscriber = new Ctor(this, func, deps);
subscribers.set(id, subscriber);
}
}

/**
* Removes the subscriber previously set with `subscribe`.
*
* @param {*} id
* @param {!SID} id
*/
unsubscribe(id) {
const subscribers = this.subscribers_;
const subscriber = subscribers && subscribers.get(id);
const subscriber = subscribers?.get(id);
if (subscriber) {
subscriber.dispose();
subscribers.delete(id);
Expand All @@ -434,8 +420,7 @@ export class ContextNode {
return;
}
const closestNode = ContextNode.closest(this.node, /* includeSelf */ false);
const parent =
(closestNode && closestNode.findGroup(this.node)) || closestNode;
const parent = closestNode?.findGroup(this.node) || closestNode;
this.updateTree_(parent, /* parentOverridden */ false);
}

Expand All @@ -453,8 +438,8 @@ export class ContextNode {
this.parent = parent;

// Remove from the old parent.
if (oldParent && oldParent.children) {
removeItem(oldParent.children, this);
if (oldParent?.children) {
removeItem(devAssert(oldParent.children), this);
}

// Add to the new parent.
Expand All @@ -466,8 +451,7 @@ export class ContextNode {
// it's other children.
// Since the new parent (`this`) is already known, this is a very
// fast operation.
for (let i = 0; i < parentChildren.length; i++) {
const child = parentChildren[i];
for (const child of parentChildren) {
if (child != this && child.isDiscoverable()) {
child.discover();
}
Expand All @@ -478,7 +462,7 @@ export class ContextNode {
}

// Check the root.
this.updateRoot(parent ? parent.root : null);
this.updateRoot(parent?.root ?? null);
}
}

Expand All @@ -498,11 +482,11 @@ function forEachContained(node, callback, includeSelf = true) {
if (closest.node == node) {
callback(closest);
} else if (closest.children) {
closest.children.forEach((child) => {
for (const child of closest.children) {
if (node.contains(child.node)) {
callback(child);
}
});
}
}
}

Expand Down
14 changes: 9 additions & 5 deletions src/context/prop.js
View file
Expand Up @@ -16,24 +16,28 @@

import {devAssert} from '../core/assert';

// typedef imports
import {ContextPropDef} from './prop.type';

const EMPTY_DEPS = [];

/**
* Creates the `ContextProp` type.
* Creates the `ContextPropDef` type.
*
* @param {string} key
* @param {{
* type: (!Object|undefined),
* deps: (!Array<!ContextProp>|undefined),
* deps: (!Array<!ContextPropDef<DEP>>|undefined),
* recursive: (boolean|(function(!Array<T>):boolean)|undefined),
* compute: ((function(!Node, !Array<T>, ...*):(T|undefined))|undefined),
* compute: (function(!Node, !Array<T>, ...DEP):(T|undefined)),
* defaultValue: (T|undefined),
* }=} opt_spec
* @return {!ContextProp<T>}
* @return {!ContextPropDef<T, DEP>}
* @template T
* @template DEP
*/
export function contextProp(key, opt_spec) {
const prop = /** @type {!ContextProp<T>} */ ({
const prop = /** @type {!ContextPropDef<T, DEP>} */ ({
key,
// Default values.
type: null,
Expand Down

0 comments on commit 8cb5da6

Please sign in to comment.