Skip to content

Commit

Permalink
♻️ Fixit: standalone assert helpers w/ format messages (#32788)
Browse files Browse the repository at this point in the history
* support formattrings in asserts

* Add tests for pure-assert

* Import as pure asserts

* Update date.js to use format args

* rename pureAssertion to assertion

* Rename `pure-assert.js` to `assert.js`

* Fix test file

* Fix copyright year

Co-authored-by: Caroline Liu <10456171+caroqliu@users.noreply.github.com>

* fix assertion

* Lint fixes

Co-authored-by: Caroline Liu <10456171+caroqliu@users.noreply.github.com>
  • Loading branch information
rcebulko and caroqliu committed Feb 23, 2021
1 parent 8fa011f commit 8c31741
Show file tree
Hide file tree
Showing 7 changed files with 255 additions and 112 deletions.
4 changes: 2 additions & 2 deletions extensions/amp-accordion/1.0/base-element.js
Expand Up @@ -23,9 +23,9 @@ import {
} from './component';
import {PreactBaseElement} from '../../../src/preact/base-element';
import {childElementsByTag, toggleAttribute} from '../../../src/dom';
import {pureDevAssert as devAssert} from '../../../src/assert';
import {dict, memo} from '../../../src/utils/object';
import {forwardRef} from '../../../src/preact/compat';
import {pureDevAssert} from '../../../src/pure-assert';
import {toArray} from '../../../src/types';
import {
useImperativeHandle,
Expand Down Expand Up @@ -175,7 +175,7 @@ function HeaderShim(
sectionElement[SECTION_POST_RENDER]();
}
return () => {
headerElement.removeEventListener('click', pureDevAssert(onClick));
headerElement.removeEventListener('click', devAssert(onClick));
};
}, [
sectionElement,
Expand Down
12 changes: 4 additions & 8 deletions extensions/amp-selector/1.0/base-element.js
Expand Up @@ -23,8 +23,8 @@ import {
toggleAttribute,
tryFocus,
} from '../../../src/dom';
import {pureDevAssert as devAssert} from '../../../src/assert';
import {dict} from '../../../src/utils/object';
import {pureDevAssert} from '../../../src/pure-assert';
import {toArray} from '../../../src/types';
import {useCallback, useLayoutEffect, useRef} from '../../../src/preact';

Expand Down Expand Up @@ -96,10 +96,7 @@ function getOptions(element, mu) {
.filter(
(el) =>
!closestAncestorElementBySelector(
pureDevAssert(
el.parentElement?.nodeType == 1 && el.parentElement,
'Expected an element'
),
devAssert(el.parentElement?.nodeType == 1, 'Expected an element'),
'[option]'
)
)
Expand Down Expand Up @@ -155,8 +152,7 @@ export function OptionShim({
return;
}
shimDomElement.addEventListener(type, handler);
return () =>
shimDomElement.removeEventListener(type, pureDevAssert(handler));
return () => shimDomElement.removeEventListener(type, devAssert(handler));
},
[shimDomElement]
);
Expand Down Expand Up @@ -241,7 +237,7 @@ function SelectorShim({
}
shimDomElement.addEventListener('keydown', onKeyDown);
return () =>
shimDomElement.removeEventListener('keydown', pureDevAssert(onKeyDown));
shimDomElement.removeEventListener('keydown', devAssert(onKeyDown));
}, [shimDomElement, onKeyDown]);

useLayoutEffect(() => {
Expand Down
172 changes: 172 additions & 0 deletions src/assert.js
@@ -0,0 +1,172 @@
/**
* 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 Dependency-free assertion helpers for use in Preact. */

/**
* Triple zero width space.
*
* This is added to user error messages, so that we can later identify
* them, when the only thing that we have is the message. This is the
* case in many browsers when the global exception handler is invoked.
*
* @const {string}
*/
export const USER_ERROR_SENTINEL = '\u200B\u200B\u200B';

/**
* User error class for use in Preact. Use of sentinel string instead of a
* boolean to check user errors because errors could be rethrown by some native
* code as a new error, and only a message would survive. Mirrors errors
* produced by `user().error()` in src/log.js.
* @final
* @public
*/
export class UserError extends Error {
/**
* Builds the error, adding the user sentinel if not present.
* @param {string} message
*/
constructor(message) {
if (!message) {
message = USER_ERROR_SENTINEL;
} else if (message.indexOf(USER_ERROR_SENTINEL) == -1) {
message += USER_ERROR_SENTINEL;
}

super(message);
}
}

/**
* Throws a provided error if the second argument isn't trueish.
* @param {Object} errorCls
* @param {T} shouldBeTruthy
* @param {string} opt_message
* @param {...*} var_args Arguments substituted into %s in the message
* @return {T}
* @throws {Error} when shouldBeTruthy is not truthy.
*/
function assertion(errorCls, shouldBeTruthy, opt_message, var_args) {
if (shouldBeTruthy) {
return shouldBeTruthy;
}

// Substitute provided values into format string in message
const message = Array.prototype.slice
// Skip the first 3 arguments to isolate format params
.call(arguments, 3)
.reduce(
(msg, subValue) => msg.replace('%s', subValue),
opt_message || 'Assertion failed'
);

throw new errorCls(message);
}

/**
* Throws a user error if the first argument isn't trueish. Mirrors userAssert
* in src/log.js.
* @param {T} shouldBeTruthy
* @param {string} opt_message
* @param {*=} opt_1 Optional argument (var arg as individual params for better
* @param {*=} opt_2 Optional argument inlining)
* @param {*=} opt_3 Optional argument
* @param {*=} opt_4 Optional argument
* @param {*=} opt_5 Optional argument
* @param {*=} opt_6 Optional argument
* @param {*=} opt_7 Optional argument
* @param {*=} opt_8 Optional argument
* @param {*=} opt_9 Optional argument
* @return {T}
* @throws {UserError} when shouldBeTruthy is not truthy.
* @closurePrimitive {asserts.truthy}
*/
export function pureUserAssert(
shouldBeTruthy,
opt_message,
opt_1,
opt_2,
opt_3,
opt_4,
opt_5,
opt_6,
opt_7,
opt_8,
opt_9
) {
return assertion(
UserError,
shouldBeTruthy,
opt_message,
opt_1,
opt_2,
opt_3,
opt_4,
opt_5,
opt_6,
opt_7,
opt_8,
opt_9
);
}

/**
* Throws an error if the first argument isn't trueish. Mirrors devAssert in
* src/log.js.
* @param {T} shouldBeTruthy
* @param {string} opt_message
* @param {*=} opt_1 Optional argument (var arg as individual params for better
* @param {*=} opt_2 Optional argument inlining)
* @param {*=} opt_3 Optional argument
* @param {*=} opt_4 Optional argument
* @param {*=} opt_5 Optional argument
* @param {*=} opt_6 Optional argument
* @param {*=} opt_7 Optional argument
* @param {*=} opt_8 Optional argument
* @param {*=} opt_9 Optional argument
* @return {T}
* @throws {Error} when shouldBeTruthy is not truthy.
* @closurePrimitive {asserts.truthy}
*/
export function pureDevAssert(
shouldBeTruthy,
opt_message,
opt_1,
opt_2,
opt_3,
opt_4,
opt_5,
opt_6,
opt_7,
opt_8,
opt_9
) {
return assertion(
Error,
shouldBeTruthy,
opt_message,
opt_1,
opt_2,
opt_3,
opt_4,
opt_5,
opt_6,
opt_7,
opt_8,
opt_9
);
}
2 changes: 1 addition & 1 deletion src/log.js
Expand Up @@ -14,7 +14,7 @@
* limitations under the License.
*/

import {USER_ERROR_SENTINEL} from './pure-assert';
import {USER_ERROR_SENTINEL} from './assert';
import {getMode} from './mode';
import {internalRuntimeVersion} from './internal-version';
import {isArray, isEnumValue} from './types';
Expand Down
94 changes: 0 additions & 94 deletions src/pure-assert.js

This file was deleted.

19 changes: 12 additions & 7 deletions src/utils/date.js
Expand Up @@ -14,7 +14,10 @@
* limitations under the License.
*/

import {pureDevAssert, pureUserAssert} from '../pure-assert';
import {
pureDevAssert as devAssert,
pureUserAssert as userAssert,
} from '../assert';

/**
* Parses the date using the `Date.parse()` rules. Additionally supports the
Expand Down Expand Up @@ -57,9 +60,9 @@ export function getDate(value) {
/** Map from attribute names to their parsers. */
const dateAttrParsers = {
'datetime': (datetime) =>
pureUserAssert(parseDate(datetime), `Invalid date: ${datetime}`),
userAssert(parseDate(datetime), 'Invalid date: %s', datetime),
'end-date': (datetime) =>
pureUserAssert(parseDate(datetime), `Invalid date: ${datetime}`),
userAssert(parseDate(datetime), 'Invalid date: %s', datetime),
'timeleft-ms': (timeleftMs) => Date.now() + Number(timeleftMs),
'timestamp-ms': (ms) => Number(ms),
'timestamp-seconds': (timestampSeconds) => 1000 * Number(timestampSeconds),
Expand All @@ -71,9 +74,10 @@ const dateAttrParsers = {
* @return {?number}
*/
export function parseDateAttrs(element, dateAttrs) {
const epoch = pureUserAssert(
const epoch = userAssert(
parseEpoch(element, dateAttrs),
`One of [${dateAttrs.join(', ')}] is required`
'One of attributes [%s] is required',
dateAttrs.join(', ')
);

const offsetSeconds =
Expand All @@ -93,9 +97,10 @@ function parseEpoch(element, dateAttrs) {
// invalid attr is provided, even if that attribute isn't present on the
// element.
const parsers = dateAttrs.map((attrName) =>
pureDevAssert(
devAssert(
dateAttrParsers[attrName],
`Invalid date attribute "${attrName}"`
'Invalid date attribute "%s"',
attrName
)
);

Expand Down

0 comments on commit 8c31741

Please sign in to comment.