Repository: sindresorhus/ow
Pull request #199 (2021-01-19 17:09:15)

Fix performance issues and cleanup code
As shown in #193, turns out, creating the stack trace needlessly is very, very taxing performance wise. With this PR, that is only done IF we're in a browser environment without `Error.captureStackTrace`. 

Also, changes some string arrays to sets, as it helps keep code cleaner and removes some if checks

sindresorhus: Can you fix the merge conflict?
vladfrangu: Done @sindresorhus!

Patch:
diff --git a/source/argument-error.ts b/source/argument-error.ts
index 4e72b97..9fb55d5 100644
--- a/source/argument-error.ts
+++ b/source/argument-error.ts
@@ -1,12 +1,14 @@
+import {generateStackTrace} from './utils/generate-stack';
+
 const wrapStackTrace = (error: ArgumentError, stack: string) => `${error.name}: ${error.message}\n${stack}`;
 
 /**
 @hidden
 */
 export class ArgumentError extends Error {
-	readonly validationErrors: ReadonlyMap<string, string[]>;
+	readonly validationErrors: ReadonlyMap<string, Set<string>>;
 
-	constructor(message: string, context: Function, stack: string, errors = new Map<string, string[]>()) {
+	constructor(message: string, context: Function, errors = new Map<string, Set<string>>()) {
 		super(message);
 
 		this.name = 'ArgumentError';
@@ -14,7 +16,7 @@ export class ArgumentError extends Error {
 		if (Error.captureStackTrace) {
 			Error.captureStackTrace(this, context);
 		} else {
-			this.stack = wrapStackTrace(this, stack);
+			this.stack = wrapStackTrace(this, generateStackTrace());
 		}
 
 		this.validationErrors = errors;
diff --git a/source/index.ts b/source/index.ts
index 3941773..7c55919 100644
--- a/source/index.ts
+++ b/source/index.ts
@@ -5,12 +5,11 @@ import {BasePredicate, isPredicate} from './predicates/base-predicate';
 import modifiers, {Modifiers} from './modifiers';
 import predicates, {Predicates} from './predicates';
 import test from './test';
-import {generateStackTrace} from './utils/generate-stack';
 
 /**
 @hidden
 */
-export type Main = <T>(value: T, label: string | Function, predicate: BasePredicate<T>, stack: string) => void;
+export type Main = <T>(value: T, label: string | Function, predicate: BasePredicate<T>) => void;
 
 // Extends is only necessary for the generated documentation to be cleaner. The loaders below infer the correct type.
 export interface Ow extends Modifiers, Predicates {
@@ -62,8 +61,6 @@ export interface ReusableValidator<T> {
 }
 
 const ow = <T>(value: T, labelOrPredicate: unknown, predicate?: BasePredicate<T>) => {
-	const stack = generateStackTrace();
-
 	if (!isPredicate(labelOrPredicate) && typeof labelOrPredicate !== 'string') {
 		throw new TypeError(`Expected second argument to be a predicate or a string, got \`${typeof labelOrPredicate}\``);
 	}
@@ -72,12 +69,12 @@ const ow = <T>(value: T, labelOrPredicate: unknown, predicate?: BasePredicate<T>
 		// If the second argument is a predicate, infer the label
 		const stackFrames = callsites();
 
-		test(value, () => inferLabel(stackFrames), labelOrPredicate, stack);
+		test(value, () => inferLabel(stackFrames), labelOrPredicate);
 
 		return;
 	}
 
-	test(value, labelOrPredicate, predicate!, stack);
+	test(value, labelOrPredicate, predicate!);
 };
 
 Object.defineProperties(ow, {
@@ -93,17 +90,15 @@ Object.defineProperties(ow, {
 	},
 	create: {
 		value: <T>(labelOrPredicate: BasePredicate<T> | string | undefined, predicate?: BasePredicate<T>) => (value: T, label?: string) => {
-			const stack = generateStackTrace();
-
 			if (isPredicate(labelOrPredicate)) {
 				const stackFrames = callsites();
 
-				test(value, label ?? (() => inferLabel(stackFrames)), labelOrPredicate, stack);
+				test(value, label ?? (() => inferLabel(stackFrames)), labelOrPredicate);
 
 				return;
 			}
 
-			test(value, label ?? (labelOrPredicate!), predicate!, stack);
+			test(value, label ?? (labelOrPredicate!), predicate!);
 		}
 	}
 });
diff --git a/source/predicates/any.ts b/source/predicates/any.ts
index 6a0ebb7..22556a4 100644
--- a/source/predicates/any.ts
+++ b/source/predicates/any.ts
@@ -13,12 +13,12 @@ export class AnyPredicate<T = unknown> implements BasePredicate<T> {
 		private readonly options: PredicateOptions = {}
 	) {}
 
-	[testSymbol](value: T, main: Main, label: string | Function, stack: string): asserts value {
-		const errors = new Map<string, string[]>();
+	[testSymbol](value: T, main: Main, label: string | Function): asserts value {
+		const errors = new Map<string, Set<string>>();
 
 		for (const predicate of this.predicates) {
 			try {
-				main(value, label, predicate, stack);
+				main(value, label, predicate);
 				return;
 			} catch (error: unknown) {
 				if (value === undefined && this.options.optional === true) {
@@ -32,13 +32,8 @@ export class AnyPredicate<T = unknown> implements BasePredicate<T> {
 						// Get the current errors set, if any.
 						const alreadyPresent = errors.get(key);
 
-						// If they are present already, create a unique set with both current and new values.
-						if (alreadyPresent) {
-							errors.set(key, [...new Set([...alreadyPresent, ...value])]);
-						} else {
-							// Add the errors found as is to the map.
-							errors.set(key, value);
-						}
+						// Add all errors under the same key
+						errors.set(key, new Set([...alreadyPresent ?? [], ...value]));
 					}
 				}
 			}
@@ -51,7 +46,6 @@ export class AnyPredicate<T = unknown> implements BasePredicate<T> {
 			throw new ArgumentError(
 				`Any predicate failed with the following errors:\n${message}`,
 				main,
-				stack,
 				errors
 			);
 		}
diff --git a/source/predicates/base-predicate.ts b/source/predicates/base-predicate.ts
index 8514044..e9c584a 100644
--- a/source/predicates/base-predicate.ts
+++ b/source/predicates/base-predicate.ts
@@ -14,5 +14,5 @@ export const isPredicate = (value: unknown): value is BasePredicate => Boolean((
 @hidden
 */
 export interface BasePredicate<T = unknown> {
-	[testSymbol](value: T, main: Main, label: string | Function, stack: string): void;
+	[testSymbol](value: T, main: Main, label: string | Function): void;
 }
diff --git a/source/predicates/predicate.ts b/source/predicates/predicate.ts
index eebdbb1..696a15b 100644
--- a/source/predicates/predicate.ts
+++ b/source/predicates/predicate.ts
@@ -96,9 +96,9 @@ export class Predicate<T = unknown> implements BasePredicate<T> {
 	/**
 	@hidden
 	*/
-	[testSymbol](value: T, main: Main, label: string | Function, stack: string): asserts value is T {
+	[testSymbol](value: T, main: Main, label: string | Function): asserts value is T {
 		// Create a map of labels -> received errors.
-		const errors = new Map<string, string[]>();
+		const errors = new Map<string, Set<string>>();
 
 		for (const {validator, message} of this.context.validators) {
 			if (this.options.optional === true && value === undefined) {
@@ -134,12 +134,10 @@ export class Predicate<T = unknown> implements BasePredicate<T> {
 			// If we already have any errors for this label.
 			if (currentErrors) {
 				// If we don't already have this error logged, add it.
-				if (!currentErrors.includes(errorMessage)) {
-					currentErrors.push(errorMessage);
-				}
+				currentErrors.add(errorMessage);
 			} else {
 				// Set this label and error in the full map.
-				errors.set(mapKey, [errorMessage]);
+				errors.set(mapKey, new Set([errorMessage]));
 			}
 		}
 
@@ -148,7 +146,7 @@ export class Predicate<T = unknown> implements BasePredicate<T> {
 			// Generate the `error.message` property.
 			const message = generateArgumentErrorMessage(errors);
 
-			throw new ArgumentError(message, main, stack, errors);
+			throw new ArgumentError(message, main, errors);
 		}
 	}
 
diff --git a/source/utils/generate-argument-error-message.ts b/source/utils/generate-argument-error-message.ts
index dad41c9..dff0692 100644
--- a/source/utils/generate-argument-error-message.ts
+++ b/source/utils/generate-argument-error-message.ts
@@ -5,19 +5,20 @@ Generates a complete message from all errors generated by predicates.
 @param isAny - If this function is called from the any argument.
 @hidden
 */
-export const generateArgumentErrorMessage = (errors: Map<string, string[]>, isAny = false) => {
+export const generateArgumentErrorMessage = (errors: Map<string, Set<string>>, isAny = false) => {
 	const message = [];
 
-	const errorArray = [...errors.values()];
+	const errorArray = [...errors.entries()];
 
-	const anyErrorWithoutOneItemOnly = errorArray.some(array => array.length !== 1);
+	const anyErrorWithoutOneItemOnly = errorArray.some(([, array]) => array.size !== 1);
 
 	// If only one error "key" is present, enumerate all of those errors only.
-	if (errors.size === 1) {
-		const returnedErrors = errorArray[0]!;
+	if (errorArray.length === 1) {
+		const [, returnedErrors] = errorArray[0]!;
 
-		if (!isAny && returnedErrors.length === 1) {
-			return returnedErrors[0]!;
+		if (!isAny && returnedErrors.size === 1) {
+			const [errorMessage] = returnedErrors;
+			return errorMessage!;
 		}
 
 		for (const entry of returnedErrors) {
@@ -29,11 +30,11 @@ export const generateArgumentErrorMessage = (errors: Map<string, string[]>, isAn
 
 	// If every predicate returns just one error, enumerate them as is.
 	if (!anyErrorWithoutOneItemOnly) {
-		return errorArray.map(([item]) => `  - ${item}`).join('\n');
+		return errorArray.map(([, [item]]) => `  - ${item}`).join('\n');
 	}
 
 	// Else, iterate through all the errors and enumerate them.
-	for (const [key, value] of errors) {
+	for (const [key, value] of errorArray) {
 		message.push(`Errors from the "${key}" predicate:`);
 		for (const entry of value) {
 			message.push(`  - ${entry}`);
diff --git a/source/utils/match-shape.ts b/source/utils/match-shape.ts
index 8890f03..80e666d 100644
--- a/source/utils/match-shape.ts
+++ b/source/utils/match-shape.ts
@@ -2,7 +2,6 @@ import is from '@sindresorhus/is';
 import test from '../test';
 import {isPredicate} from '../predicates/base-predicate';
 import {BasePredicate} from '..';
-import {generateStackTrace} from './generate-stack';
 
 // eslint-disable-next-line @typescript-eslint/consistent-indexed-object-style
 export interface Shape {
@@ -45,13 +44,12 @@ Test if the `object` matches the `shape` partially.
 @param parent - Name of the parent property.
 */
 export function partial(object: Record<string, any>, shape: Shape, parent?: string): boolean | string {
-	const stack = generateStackTrace();
 	try {
 		for (const key of Object.keys(shape)) {
 			const label = parent ? `${parent}.${key}` : key;
 
 			if (isPredicate(shape[key])) {
-				test(object[key], label, shape[key] as BasePredicate, stack);
+				test(object[key], label, shape[key] as BasePredicate);
 			} else if (is.plainObject(shape[key])) {
 				const result = partial(object[key], shape[key] as Shape, label);
 
@@ -77,7 +75,6 @@ Test if the `object` matches the `shape` exactly.
 @param parent - Name of the parent property.
 */
 export function exact(object: Record<string, any>, shape: Shape, parent?: string, isArray?: boolean): boolean | string {
-	const stack = generateStackTrace();
 	try {
 		const objectKeys = new Set<string>(Object.keys(object));
 
@@ -87,7 +84,7 @@ export function exact(object: Record<string, any>, shape: Shape, parent?: string
 			const label = parent ? `${parent}.${key}` : key;
 
 			if (isPredicate(shape[key])) {
-				test(object[key], label, shape[key] as BasePredicate, stack);
+				test(object[key], label, shape[key] as BasePredicate);
 			} else if (is.plainObject(shape[key])) {
 				if (!Object.prototype.hasOwnProperty.call(object, key)) {
 					return `Expected \`${label}\` to exist`;


Test patch:
diff --git a/source/test.ts b/source/test.ts
index 699d5c4..64795d7 100644
--- a/source/test.ts
+++ b/source/test.ts
@@ -9,6 +9,6 @@ Validate the value against the provided predicate.
 @param label - Label which should be used in error messages.
 @param predicate - Predicate to test to value against.
 */
-export default function test<T>(value: T, label: string | Function, predicate: BasePredicate<T>, stack: string) {
-	predicate[testSymbol](value, test, label, stack);
+export default function test<T>(value: T, label: string | Function, predicate: BasePredicate<T>) {
+	predicate[testSymbol](value, test, label);
 }
diff --git a/test/any-multiple-errors.ts b/test/any-multiple-errors.ts
index 8842f60..f84cbf4 100644
--- a/test/any-multiple-errors.ts
+++ b/test/any-multiple-errors.ts
@@ -13,10 +13,10 @@ test('any predicate', t => {
 
 	const reportedError_1_1 = error_1.validationErrors.get('string')!;
 
-	t.is(reportedError_1_1.length, 1, 'There should be only one element');
-	t.deepEqual(reportedError_1_1, [
+	t.is(reportedError_1_1.size, 1, 'There should be only one element');
+	t.deepEqual(reportedError_1_1, new Set([
 		'Expected argument to be of type `string` but received type `number`'
-	]);
+	]));
 
 	// #endregion
 
@@ -43,18 +43,18 @@ test('any predicate', t => {
 	const reportedError_2_1 = error_2.validationErrors.get('string')!;
 	const reportedError_2_2 = error_2.validationErrors.get('number')!;
 
-	t.is(reportedError_2_1.length, 3, 'There should be three errors reported for the string predicate');
-	t.is(reportedError_2_2.length, 1, 'There should be one error reported for the number predicate');
+	t.is(reportedError_2_1.size, 3, 'There should be three errors reported for the string predicate');
+	t.is(reportedError_2_2.size, 1, 'There should be one error reported for the number predicate');
 
-	t.deepEqual(reportedError_2_1, [
+	t.deepEqual(reportedError_2_1, new Set([
 		'Expected argument to be of type `string` but received type `number`',
 		'Expected string to be a URL, got `21`',
 		'Expected string to have a minimum length of `24`, got `21`'
-	]);
+	]));
 
-	t.deepEqual(reportedError_2_2, [
+	t.deepEqual(reportedError_2_2, new Set([
 		'Expected number to be greater than 42, got 21'
-	]);
+	]));
 
 	// #endregion
 
@@ -74,15 +74,15 @@ test('any predicate', t => {
 	const reportedError_3_1 = error_3.validationErrors.get('string')!;
 	const reportedError_3_2 = error_3.validationErrors.get('number')!;
 
-	t.is(reportedError_3_1.length, 1, 'There should be one error reported for the string predicate');
-	t.is(reportedError_3_2.length, 1, 'There should be one error reported for the number predicate');
+	t.is(reportedError_3_1.size, 1, 'There should be one error reported for the string predicate');
+	t.is(reportedError_3_2.size, 1, 'There should be one error reported for the number predicate');
 
-	t.deepEqual(reportedError_3_1, [
+	t.deepEqual(reportedError_3_1, new Set([
 		'Expected argument to be of type `string` but received type `null`'
-	]);
-	t.deepEqual(reportedError_3_2, [
+	]));
+	t.deepEqual(reportedError_3_2, new Set([
 		'Expected argument to be of type `number` but received type `null`'
-	]);
+	]));
 
 	const error_4 = t.throws<ArgumentError>(() => {
 		ow(21 as any, ow.any(
@@ -100,19 +100,19 @@ test('any predicate', t => {
 
 	const reportedError_4_1 = error_4.validationErrors.get('string')!;
 
-	t.is(reportedError_4_1.length, 4, 'There should be four errors reported for the string predicate');
+	t.is(reportedError_4_1.size, 4, 'There should be four errors reported for the string predicate');
 
-	t.deepEqual(reportedError_4_1, [
+	t.deepEqual(reportedError_4_1, new Set([
 		'Expected argument to be of type `string` but received type `number`',
 		'Expected string to be a URL, got `21`',
 		'Expected string to have a minimum length of `21`, got `21`',
 		'Expected string to have a minimum length of `42`, got `21`'
-	]);
+	]));
 	// #endregion
 
 	// #region Tests line 47,65
 	class CustomPredicate implements BasePredicate<string> {
-		[testSymbol](_value: string, _main: Main, _label: string | Function, _stack: string): void {
+		[testSymbol](_value: string, _main: Main, _label: string | Function): void {
 			throw new Error('Custom error.');
 		}
 	}
diff --git a/test/custom-message.ts b/test/custom-message.ts
index d42d938..530239e 100644
--- a/test/custom-message.ts
+++ b/test/custom-message.ts
@@ -39,11 +39,11 @@ test('custom validate message', t => {
 
 	const result1_ = error.validationErrors.get('string')!;
 
-	t.is(result1_.length, 2, 'There are two reported errors for this input');
-	t.deepEqual(result1_, [
+	t.is(result1_.size, 2, 'There are two reported errors for this input');
+	t.deepEqual(result1_, new Set([
 		'Expected string, to be have a minimum length of 5, got `1234`',
 		'This is no url'
-	], 'There is an error for the string length, and one for invalid URL');
+	]), 'There is an error for the string length, and one for invalid URL');
 
 	t.throws(() => {
 		ow('12345', ow.string.minLength(5).message((value, label) => `Expected ${label}, to be have a minimum length of 5, got \`${value}\``).url.message('This is no url'));
diff --git a/test/test.ts b/test/test.ts
index 7210b91..b4d6933 100644
--- a/test/test.ts
+++ b/test/test.ts
@@ -221,6 +221,16 @@ test('not', t => {
 		ow(new ArrayBuffer(2), ow.arrayBuffer.not.maxByteLength(2));
 	}, 'Expected ArrayBuffer to have a minimum byte length of `3`, got `2`');
 
+	t.notThrows(() => {
+		ow(new SharedArrayBuffer(4), ow.sharedArrayBuffer.not.maxByteLength(2));
+	});
+	t.notThrows(() => {
+		ow(new SharedArrayBuffer(3), ow.sharedArrayBuffer.not.maxByteLength(2));
+	});
+	t.throws(() => {
+		ow(new SharedArrayBuffer(2), ow.sharedArrayBuffer.not.maxByteLength(2));
+	}, 'Expected SharedArrayBuffer to have a minimum byte length of `3`, got `2`');
+
 	t.notThrows(() => {
 		ow(new DataView(new ArrayBuffer(1)), ow.dataView.not.minByteLength(3));
 	});
@@ -312,11 +322,11 @@ test('reusable validator', t => {
 
 	const result1_ = error.validationErrors.get('string')!;
 
-	t.is(result1_.length, 2, 'There are two reported errors for this input');
-	t.deepEqual(result1_, [
+	t.is(result1_.size, 2, 'There are two reported errors for this input');
+	t.deepEqual(result1_, new Set([
 		'Expected argument to be of type `string` but received type `number`',
 		'Expected string to have a minimum length of `3`, got `5`'
-	], 'There is an error for invalid input type, and one for minimum length not being satisfied');
+	]), 'There is an error for invalid input type, and one for minimum length not being satisfied');
 });
 
 test('reusable validator called with label', t => {
@@ -353,11 +363,11 @@ test('reusable validator called with label', t => {
 
 	const result1_ = error.validationErrors.get('bar')!;
 
-	t.is(result1_.length, 2, 'There are two reported errors for this input');
-	t.deepEqual(result1_, [
+	t.is(result1_.size, 2, 'There are two reported errors for this input');
+	t.deepEqual(result1_, new Set([
 		'Expected `bar` to be of type `string` but received type `number`',
 		'Expected string `bar` to have a minimum length of `3`, got `5`'
-	], 'There is an error for invalid input type, and one for minimum length not being satisfied');
+	]), 'There is an error for invalid input type, and one for minimum length not being satisfied');
 });
 
 test('reusable validator with label', t => {
@@ -387,11 +397,11 @@ test('reusable validator with label', t => {
 
 	const result1_ = error.validationErrors.get('foo')!;
 
-	t.is(result1_.length, 2, 'There are two reported errors for this input');
-	t.deepEqual(result1_, [
+	t.is(result1_.size, 2, 'There are two reported errors for this input');
+	t.deepEqual(result1_, new Set([
 		'Expected `foo` to be of type `string` but received type `number`',
 		'Expected string `foo` to have a minimum length of `3`, got `5`'
-	], 'There is an error for invalid input type, and one for minimum length not being satisfied');
+	]), 'There is an error for invalid input type, and one for minimum length not being satisfied');
 });
 
 test('reusable validator with label called with label', t => {
@@ -423,11 +433,11 @@ test('reusable validator with label called with label', t => {
 
 	const result1_ = error.validationErrors.get('bar')!;
 
-	t.is(result1_.length, 2, 'There are two reported errors for this input');
-	t.deepEqual(result1_, [
+	t.is(result1_.size, 2, 'There are two reported errors for this input');
+	t.deepEqual(result1_, new Set([
 		'Expected `bar` to be of type `string` but received type `number`',
 		'Expected string `bar` to have a minimum length of `3`, got `5`'
-	], 'There is an error for invalid input type, and one for minimum length not being satisfied');
+	]), 'There is an error for invalid input type, and one for minimum length not being satisfied');
 });
 
 test('any-reusable validator', t => {
@@ -513,3 +523,16 @@ test('ow without Error.captureStackTrace', t => {
 		value: true
 	});
 });
+
+// This test is to cover all paths of source/argument-error.ts
+test('ArgumentError with missing errors map', t => {
+	function throws() {
+		throw new ArgumentError('Hi from tests!', throws);
+	}
+
+	const error_1 = t.throws<ArgumentError>(() => {
+		throws();
+	}, 'Hi from tests!');
+
+	t.deepEqual(error_1.validationErrors, new Map(), 'Error should default to an empty error map');
+});