Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

feat: switch from clone to built in structured clone #9068

Merged
merged 4 commits into from
Aug 31, 2023
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
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