Skip to content

Commit

Permalink
feat: switch from clone to built in structured clone (vega#9068)
Browse files Browse the repository at this point in the history
  • Loading branch information
domoritz authored and alliefeldman committed Sep 30, 2023
1 parent 9dbef23 commit 600e907
Show file tree
Hide file tree
Showing 15 changed files with 998 additions and 1,202 deletions.
13 changes: 5 additions & 8 deletions package.json
Original file line number Diff line number Diff line change
Expand Up @@ -92,7 +92,7 @@
"@rollup/plugin-terser": "^0.4.3",
"@types/chai": "^4.3.5",
"@types/d3": "^7.4.0",
"@types/jest": "^27.4.1",
"@types/jest": "^29.5.4",
"@types/pako": "^2.0.0",
"@typescript-eslint/eslint-plugin": "^6.4.1",
"@typescript-eslint/parser": "^6.4.1",
Expand All @@ -107,9 +107,10 @@
"eslint-config-prettier": "^9.0.0",
"eslint-plugin-jest": "^27.2.3",
"eslint-plugin-prettier": "^5.0.0",
"fast-json-stable-stringify": "~2.1.0",
"highlight.js": "^11.8.0",
"jest": "^27.5.1",
"jest-dev-server": "^6.1.1",
"jest": "^29.6.3",
"jest-dev-server": "^9.0.0",
"mkdirp": "^3.0.1",
"pako": "^2.1.0",
"prettier": "^3.0.2",
Expand All @@ -130,10 +131,6 @@
"yaml-front-matter": "^4.1.1"
},
"dependencies": {
"@types/clone": "~2.1.1",
"clone": "~2.1.2",
"fast-deep-equal": "~3.1.3",
"fast-json-stable-stringify": "~2.1.0",
"json-stringify-pretty-compact": "~3.0.0",
"tslib": "~2.6.2",
"vega-event-selector": "~3.0.1",
Expand All @@ -145,6 +142,6 @@
"vega": "^5.24.0"
},
"engines": {
"node": ">=16"
"node": ">=18"
}
}
2 changes: 0 additions & 2 deletions rollup.config.mjs
Original file line number Diff line number Diff line change
@@ -1,6 +1,5 @@
import alias from '@rollup/plugin-alias';
import babel from '@rollup/plugin-babel';
import commonjs from '@rollup/plugin-commonjs';
import json from '@rollup/plugin-json';
import resolve from '@rollup/plugin-node-resolve';
import terser from '@rollup/plugin-terser';
Expand Down Expand Up @@ -67,7 +66,6 @@ const outputs = [
}
}),
resolve({browser: true, extensions}),
commonjs(),
json(),
babel({
extensions,
Expand Down
135 changes: 122 additions & 13 deletions src/util.ts
Original file line number Diff line number Diff line change
@@ -1,11 +1,7 @@
import {default as clone_} from 'clone';
import deepEqual_ from 'fast-deep-equal';
import stableStringify from 'fast-json-stable-stringify';
import {hasOwnProperty, isNumber, isString, splitAccessPath, stringValue, writeConfig} from 'vega-util';
import {isLogicalAnd, isLogicalNot, isLogicalOr, LogicalComposition} from './logical';

export const deepEqual = deepEqual_;
export const duplicate = clone_;
export const duplicate = structuredClone;

export function never(message: string): never {
throw new Error(message);
Expand Down Expand Up @@ -46,14 +42,9 @@ export function omit<T extends object, K extends keyof T>(obj: T, props: readonl
* Monkey patch Set so that `stringify` produces a string representation of sets.
*/
Set.prototype['toJSON'] = function () {
return `Set(${[...this].map(x => stableStringify(x)).join(',')})`;
return `Set(${[...this].map(x => stringify(x)).join(',')})`;
};

/**
* Converts any object to a string representation that can be consumed by humans.
*/
export const stringify = stableStringify;

/**
* Converts any object to a string of limited size, or a number.
*/
Expand All @@ -62,7 +53,7 @@ export function hash(a: any): string | number {
return a;
}

const str = isString(a) ? a : stableStringify(a);
const str = isString(a) ? a : stringify(a);

// short strings can be used as hash directly, longer strings are hashed to reduce memory usage
if (str.length < 250) {
Expand Down Expand Up @@ -152,7 +143,7 @@ export function unique<T>(values: readonly T[], f: (item: T) => string | number)
export type Dict<T> = Record<string, T>;

/**
* Returns true if the two dictionaries disagree. Applies only to defined values.
* Returns true if the two dictionaries agree. Applies only to defined values.
*/
export function isEqual<T>(dict: Dict<T>, other: Dict<T>) {
const dictKeys = keys(dict);
Expand Down Expand Up @@ -400,3 +391,121 @@ export function isNumeric(value: number | string): boolean {
}
return !isNaN(value as any) && !isNaN(parseFloat(value));
}

const clonedProto = Object.getPrototypeOf(structuredClone({}));

/**
* Compares two values for equality, including arrays and objects.
*
* Adapted from https://github.com/epoberezkin/fast-deep-equal.
*/
export function deepEqual(a: any, b: any) {
if (a === b) return true;

if (a && b && typeof a == 'object' && typeof b == 'object') {
// compare names to avoid issues with structured clone
if (a.constructor.name !== b.constructor.name) return false;

let length;
let i;

if (Array.isArray(a)) {
length = a.length;
if (length != b.length) return false;
for (i = length; i-- !== 0; ) if (!deepEqual(a[i], b[i])) return false;
return true;
}

if (a instanceof Map && b instanceof Map) {
if (a.size !== b.size) return false;
for (i of a.entries()) if (!b.has(i[0])) return false;
for (i of a.entries()) if (!deepEqual(i[1], b.get(i[0]))) return false;
return true;
}

if (a instanceof Set && b instanceof Set) {
if (a.size !== b.size) return false;
for (i of a.entries()) if (!b.has(i[0])) return false;
return true;
}

if (ArrayBuffer.isView(a) && ArrayBuffer.isView(b)) {
length = (a as any).length;
if (length != (b as any).length) return false;
for (i = length; i-- !== 0; ) if (a[i] !== b[i]) return false;
return true;
}

if (a.constructor === RegExp) return a.source === b.source && a.flags === b.flags;
// also compare to structured clone prototype
if (a.valueOf !== Object.prototype.valueOf && a.valueOf !== clonedProto.valueOf) return a.valueOf() === b.valueOf();
if (a.toString !== Object.prototype.toString && a.toString !== clonedProto.toString)
return a.toString() === b.toString();

const ks = Object.keys(a);
length = ks.length;
if (length !== Object.keys(b).length) return false;

for (i = length; i-- !== 0; ) if (!Object.prototype.hasOwnProperty.call(b, ks[i])) return false;

for (i = length; i-- !== 0; ) {
const key = ks[i];

if (!deepEqual(a[key], b[key])) return false;
}

return true;
}

// true if both NaN, false otherwise
return a !== a && b !== b;
}

/**
* Converts any object to a string representation that can be consumed by humans.
*
* Adapted from https://github.com/epoberezkin/fast-json-stable-stringify
*/
export function stringify(data: any) {
const seen: any[] = [];

return (function _stringify(node: any) {
if (node && node.toJSON && typeof node.toJSON === 'function') {
node = node.toJSON();
}

if (node === undefined) return undefined;
if (typeof node == 'number') return isFinite(node) ? '' + node : 'null';
if (typeof node !== 'object') return JSON.stringify(node);

let i, out;
if (Array.isArray(node)) {
out = '[';
for (i = 0; i < node.length; i++) {
if (i) out += ',';
out += _stringify(node[i]) || 'null';
}
return out + ']';
}

if (node === null) return 'null';

if (seen.includes(node)) {
throw new TypeError('Converting circular structure to JSON');
}

const seenIndex = seen.push(node) - 1;
const ks = Object.keys(node).sort();
out = '';
for (i = 0; i < ks.length; i++) {
const key = ks[i];
const value = _stringify(node[key]);

if (!value) continue;
if (out) out += ',';
out += JSON.stringify(key) + ':' + value;
}
seen.splice(seenIndex, 1);
return `{${out}}`;
})(data);
}
15 changes: 15 additions & 0 deletions test-runtime/babel.config.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,15 @@
// Only used for jest
module.exports = {
presets: [
[
'@babel/preset-env',
{
targets: {
node: 'current'
}
}
],
'@babel/preset-typescript'
],
plugins: ['@babel/proposal-class-properties']
};
2 changes: 1 addition & 1 deletion test-runtime/discrete.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,7 @@ describe(`point selections at runtime in unit views`, () => {
let testRender: (filename: string) => Promise<void>;

beforeAll(async () => {
page = await (global as any).__BROWSER__.newPage();
page = await (global as any).__BROWSER_GLOBAL__.newPage();
embed = embedFn(page);
testRender = testRenderFn(page, `${type}/unit`);
await page.goto('http://0.0.0.0:8000/test-runtime/');
Expand Down
2 changes: 1 addition & 1 deletion test-runtime/interval.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,7 @@ describe('interval selections at runtime in unit views', () => {
let testRender: (filename: string) => Promise<void>;

beforeAll(async () => {
page = await (global as any).__BROWSER__.newPage();
page = await (global as any).__BROWSER_GLOBAL__.newPage();
embed = embedFn(page);
testRender = testRenderFn(page, `${type}/unit`);
await page.goto('http://0.0.0.0:8000/test-runtime/');
Expand Down
15 changes: 9 additions & 6 deletions test-runtime/puppeteer_environment.js
Original file line number Diff line number Diff line change
@@ -1,8 +1,8 @@
const fs = require('fs');
const {readFile} = require('fs').promises;
const os = require('os');
const path = require('path');
const puppeteer = require('puppeteer');
const NodeEnvironment = require('jest-environment-node');
const NodeEnvironment = require('jest-environment-node').TestEnvironment;

const DIR = path.join(os.tmpdir(), 'jest_puppeteer_global_setup');

Expand All @@ -14,23 +14,26 @@ class PuppeteerEnvironment extends NodeEnvironment {
async setup() {
await super.setup();
// get the wsEndpoint
const wsEndpoint = fs.readFileSync(path.join(DIR, 'wsEndpoint'), 'utf8');
const wsEndpoint = await readFile(path.join(DIR, 'wsEndpoint'), 'utf8');
if (!wsEndpoint) {
throw new Error('wsEndpoint not found');
}

// connect to puppeteer
this.global.__BROWSER__ = await puppeteer.connect({
this.global.__BROWSER_GLOBAL__ = await puppeteer.connect({
browserWSEndpoint: wsEndpoint
});
}

async teardown() {
if (this.global.__BROWSER_GLOBAL__) {
this.global.__BROWSER_GLOBAL__.disconnect();
}
await super.teardown();
}

runScript(script) {
return super.runScript(script);
getVmContext() {
return super.getVmContext();
}
}

Expand Down
2 changes: 1 addition & 1 deletion test-runtime/resolve.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -24,7 +24,7 @@ for (const type of selectionTypes) {
let embed: (specification: TopLevelSpec) => Promise<void>;

beforeAll(async () => {
page = await (global as any).__BROWSER__.newPage();
page = await (global as any).__BROWSER_GLOBAL__.newPage();
embed = embedFn(page);
await page.goto('http://0.0.0.0:8000/test-runtime/');
});
Expand Down
11 changes: 5 additions & 6 deletions test-runtime/setup.js
Original file line number Diff line number Diff line change
@@ -1,15 +1,14 @@
const fs = require('fs');
const {mkdir, writeFile} = require('fs').promises;
const os = require('os');
const path = require('path');
const mkdirp = require('mkdirp');
const puppeteer = require('puppeteer');
const {setup: setupDevServer} = require('jest-dev-server');
const chalk = require('chalk');

const DIR = path.join(os.tmpdir(), 'jest_puppeteer_global_setup');

module.exports = async function () {
await setupDevServer({
globalThis.servers = await setupDevServer({
command: `node ./node_modules/.bin/serve -l 8000`,
launchTimeout: 50000,
port: 8000
Expand All @@ -20,9 +19,9 @@ module.exports = async function () {
const browser = await puppeteer.launch();
// store the browser instance so we can teardown it later
// this global is only available in the teardown but not in TestEnvironments
global.__BROWSER_GLOBAL__ = browser;
globalThis.__BROWSER_GLOBAL__ = browser;

// use the file system to expose the wsEndpoint for TestEnvironments
mkdirp.sync(DIR);
fs.writeFileSync(path.join(DIR, 'wsEndpoint'), browser.wsEndpoint());
await mkdir(DIR, {recursive: true});
await writeFile(path.join(DIR, 'wsEndpoint'), browser.wsEndpoint());
};
9 changes: 5 additions & 4 deletions test-runtime/teardown.js
Original file line number Diff line number Diff line change
@@ -1,15 +1,16 @@
const fs = require('fs').promises;
const os = require('os');
const path = require('path');
const rimraf = require('rimraf');
const {teardown: teardownDevServer} = require('jest-dev-server');

const DIR = path.join(os.tmpdir(), 'jest_puppeteer_global_setup');

module.exports = async function () {
await teardownDevServer();
await teardownDevServer(globalThis.servers);

// close the browser instance
await global.__BROWSER_GLOBAL__.close();
await globalThis.__BROWSER_GLOBAL__.close();

// clean-up the wsEndpoint file
rimraf.sync(DIR);
await fs.rm(DIR, {recursive: true, force: true});
};
2 changes: 1 addition & 1 deletion test-runtime/toggle.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -23,7 +23,7 @@ describe('Toggle point selections at runtime', () => {
let testRender: (filename: string) => Promise<void>;

beforeAll(async () => {
page = await (global as any).__BROWSER__.newPage();
page = await (global as any).__BROWSER_GLOBAL__.newPage();
embed = embedFn(page);
testRender = testRenderFn(page, 'point/toggle');
await page.goto('http://0.0.0.0:8000/test-runtime/');
Expand Down
2 changes: 1 addition & 1 deletion test-runtime/translate.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -23,7 +23,7 @@ describe('Translate interval selections at runtime', () => {
let testRender: (filename: string) => Promise<void>;

beforeAll(async () => {
page = await (global as any).__BROWSER__.newPage();
page = await (global as any).__BROWSER_GLOBAL__.newPage();
embed = embedFn(page);
await page.goto('http://0.0.0.0:8000/test-runtime/');
});
Expand Down
2 changes: 1 addition & 1 deletion test-runtime/zoom.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -33,7 +33,7 @@ describe('Zoom interval selections at runtime', () => {
let testRender: (filename: string) => Promise<void>;

beforeAll(async () => {
page = await (global as any).__BROWSER__.newPage();
page = await (global as any).__BROWSER_GLOBAL__.newPage();
embed = embedFn(page);
await page.goto('http://0.0.0.0:8000/test-runtime/');
});
Expand Down
Loading

0 comments on commit 600e907

Please sign in to comment.