diff --git a/lib/serialize/other.js b/lib/serialize/other.js index 46f538e3..68c90a50 100644 --- a/lib/serialize/other.js +++ b/lib/serialize/other.js @@ -9,7 +9,8 @@ const t = require('@babel/types'); // Imports -const { +const {createDependency, deleteDependency} = require('./records.js'), + { REGEXP_TYPE, DATE_TYPE, URL_TYPE, URL_SEARCH_PARAMS_TYPE, registerSerializer } = require('./types.js'), {URLContextSymbol, URLQuerySymbol} = require('../shared/globals.js'); @@ -20,6 +21,7 @@ const regexSourceGetter = Object.getOwnPropertyDescriptor(RegExp.prototype, 'sou regexFlagsGetter = Object.getOwnPropertyDescriptor(RegExp.prototype, 'flags').get, dateGetTime = Date.prototype.getTime, URLToString = URL.prototype.toString, + URLSearchParamsGetter = Object.getOwnPropertyDescriptor(URL.prototype, 'searchParams').get, URLSearchParamsToString = URLSearchParams.prototype.toString; const urlShouldSkipKey = URLContextSymbol @@ -62,9 +64,22 @@ module.exports = { * @returns {Function} - Serializer function */ traceURL(url, record) { - // TODO: Record relationship to its SearchParams so serializing - // `const u = new URL('http://foo.com/?x=1'); return {u, s: u.searchParams}` - // recognises the 2 as related in NodeJS v20. + // Record relationship to URL's SearchParams. + // This ensures `const u = new URL('http://foo.com/?x=1'); return {u, s: u.searchParams}` + // recognises the URL and its SearchParams as related. + // Allow `URLSearchParamsGetter.call(url)` to fail as in NodeJS v18, could be a non-URL object + // which has had prototype set to `URL.prototype`. + // TODO: This implementation isn't quite right on NodeJS v18 as URL objects have + // symbol properties which we're ignoring. In NodeJS v20 these properties no longer exist. + let params; + try { params = URLSearchParamsGetter.call(url); } catch {} // eslint-disable-line no-empty + if (params) { + const paramsRecord = this.traceValue(params, 'searchParams', '.searchParams'); + createDependency(paramsRecord, record); + paramsRecord.extra.urlRecord = record; + this.maybeSoloURLSearchParams.push(paramsRecord); + } + this.traceProperties(url, record, urlShouldSkipKey); record.extra = {url: URLToString.call(url)}; return URL_TYPE; @@ -79,22 +94,23 @@ module.exports = { * @returns {Function} - Serializer function */ traceURLSearchParams(params, record) { - // `url[Symbol('context')]` property was removed in NodeJS v20.0.0 - const url = URLContextSymbol ? params[URLContextSymbol] : null; - if (url) { - // Has URL context - `new URL(...).searchParams` - const urlRecord = this.traceDependency(url, 'url', '[Symbol(URLContext)]', record); - record.extra = {params: null, urlRecord}; - } else { - // Has no context - `new URLSearchParams(...)` - record.extra = {params: URLSearchParamsToString.call(params), urlRecord: null}; - } - + record.extra = {params: URLSearchParamsToString.call(params), urlRecord: null}; this.traceProperties(params, record, urlShouldSkipKey); - return URL_SEARCH_PARAMS_TYPE; }, + /** + * Remove any `URLSearchParams` which have no dependents. + * `traceURL()` adds a dependency from the URL's search params on the URL, + * but if the search params aren't referenced in their own right, this dependency should be removed. + * @returns {undefined} + */ + finalizeURLSearchParamses() { + for (const paramsRecord of this.maybeSoloURLSearchParams) { + if (!paramsRecord.dependents) deleteDependency(paramsRecord, paramsRecord.extra.urlRecord); + } + }, + traceWeakRef(weakRef, record) { // eslint-disable-line no-unused-vars // TODO throw new Error('Cannot serialize WeakRefs'); @@ -175,7 +191,7 @@ registerSerializer(URL_TYPE, serializeURL); */ function serializeURLSearchParams(record) { const node = record.extra.urlRecord - // `new URL(...).searchParams` + // `url.searchParams` ? t.memberExpression(this.serializeValue(record.extra.urlRecord), t.identifier('searchParams')) // `new URLSearchParams(...)` : t.newExpression( diff --git a/lib/serialize/serialize.js b/lib/serialize/serialize.js index 55c6357e..b0b1753f 100644 --- a/lib/serialize/serialize.js +++ b/lib/serialize/serialize.js @@ -136,6 +136,9 @@ module.exports = { output = createEntryOutput(SYNC_SPLIT_POINT, undefined, record, name); } + // Finalize trace - remove orphans etc + this.finalizeTrace(); + return {pointOutputs, entryPointOutputs, entryAndAsyncSplitPointOutputs}; }, diff --git a/lib/serialize/trace.js b/lib/serialize/trace.js index eb577503..733b77ad 100644 --- a/lib/serialize/trace.js +++ b/lib/serialize/trace.js @@ -46,6 +46,8 @@ module.exports = { this.urlSearchParamsPrototypeRecord = this.traceValue(URLSearchParams.prototype, null, null); this.minusZeroRecord = null; + + this.maybeSoloURLSearchParams = []; }, /** @@ -129,11 +131,12 @@ module.exports = { // Trace object assertBug(typeof val === 'object'); - // TODO Add support for remaining types commented out below + // TODO: Add support for remaining types commented out below const objType = typeOf(val); if (objType === 'Object') { // `URL` and `URLSearchParams` are implemented in Javascript in Node internals // and so cannot be detected with a type check + // TODO: I think they can now be identified with a type check in NodeJS v18+ if (val instanceof URL) return this.traceURL(val, record); if (val instanceof URLSearchParams) return this.traceURLSearchParams(val, record); return this.traceObject(val, record); @@ -190,6 +193,15 @@ module.exports = { */ traceAndSerializeGlobal(val) { return this.serializeValue(this.traceValue(val, null, null)); + }, + + /** + * Finalize trace. + * Tidy up any orphans etc. + * @returns {undefined} + */ + finalizeTrace() { + this.finalizeURLSearchParamses(); } }; diff --git a/package-lock.json b/package-lock.json index ae8b0d07..70057ff3 100644 --- a/package-lock.json +++ b/package-lock.json @@ -54,7 +54,6 @@ "jest-runner-eslint": "^2.1.1", "jest-util": "^29.6.3", "npm-run-all": "^4.1.5", - "parse-node-version": "^2.0.0", "supports-color": "^9.4.0", "tinypool": "^0.8.0" }, @@ -7822,15 +7821,6 @@ "url": "https://github.com/sponsors/sindresorhus" } }, - "node_modules/parse-node-version": { - "version": "2.0.0", - "resolved": "https://registry.npmjs.org/parse-node-version/-/parse-node-version-2.0.0.tgz", - "integrity": "sha512-/jc2J3E6IazoQF4RlwJg4E4FByHqXlcoaHVrWQvjr7LEQd8QVLBT3AO5dqZlFPIzluYUosVTLAM0t8OTEc2AdQ==", - "dev": true, - "engines": { - "node": ">= 10.13.0" - } - }, "node_modules/path-exists": { "version": "4.0.0", "resolved": "https://registry.npmjs.org/path-exists/-/path-exists-4.0.0.tgz", diff --git a/package.json b/package.json index 2e1de9d3..b5636728 100644 --- a/package.json +++ b/package.json @@ -77,7 +77,6 @@ "jest-runner-eslint": "^2.1.1", "jest-util": "^29.6.3", "npm-run-all": "^4.1.5", - "parse-node-version": "^2.0.0", "supports-color": "^9.4.0", "tinypool": "^0.8.0" }, diff --git a/test/other.test.js b/test/other.test.js index 6cb03ccd..fad8fc86 100644 --- a/test/other.test.js +++ b/test/other.test.js @@ -5,18 +5,11 @@ 'use strict'; -// Modules -const parseNodeVersion = require('parse-node-version'); - // Imports const {itSerializes, itSerializesEqual} = require('./support/index.js'); // Tests -// `url[Symbol('context')]` property was removed in NodeJS v20.0.0. -const urlsHaveContext = parseNodeVersion(process.version).major < 20, - itSerializesEqualIfUrlsHaveContext = urlsHaveContext ? itSerializesEqual : itSerializesEqual.skip; - describe('RegExps', () => { itSerializesEqual('with no flags', { in: () => /^foo$/, @@ -138,7 +131,7 @@ describe('Dates', () => { }); describe('URLs', () => { - itSerializesEqual('URL', { + itSerializes('URL', { in: () => new URL('http://foo.com/path/to/file.html?a=1&b=2'), out: 'new URL("http://foo.com/path/to/file.html?a=1&b=2")', validate(url) { @@ -175,7 +168,7 @@ describe('URLs', () => { }); describe('URLSearchParams', () => { - itSerializesEqual('without context', { + itSerializesEqual('without accompanying URL', { in: () => new URLSearchParams('a=1&b=2'), out: 'new URLSearchParams("a=1&b=2")', validate(params) { @@ -184,23 +177,42 @@ describe('URLSearchParams', () => { } }); - // This test only makes sense in NodeJS v18. - // `url[Symbol('context')]` was removed in NodeJS v20.0.0. - itSerializesEqualIfUrlsHaveContext('with context', { - in: () => new URL('http://foo.com/path/to/file.html?a=1&b=2').searchParams, - out: 'new URL("http://foo.com/path/to/file.html?a=1&b=2").searchParams', - /* eslint-disable jest/no-standalone-expect */ - validate(params) { - expect(params).toBeInstanceOf(URLSearchParams); - expect(params.toString()).toBe('a=1&b=2'); + describe('with URL', () => { + itSerializesEqual('URL traced first', { + in() { + const url = new URL('http://foo.com/path/to/file.html?a=1&b=2'); + return {url, params: url.searchParams}; + }, + out: `(()=>{ + const a=new URL("http://foo.com/path/to/file.html?a=1&b=2"); + return{url:a,params:a.searchParams} + })()`, + validate({url, params}) { + expect(url).toBeInstanceOf(URL); + expect(url.toString()).toBe('http://foo.com/path/to/file.html?a=1&b=2'); + expect(params).toBeInstanceOf(URLSearchParams); + expect(params.toString()).toBe('a=1&b=2'); + expect(params).toBe(url.searchParams); + } + }); - const contextSymbol = Object.getOwnPropertySymbols(params)[1]; - expect(contextSymbol.toString()).toBe('Symbol(context)'); - const url = params[contextSymbol]; - expect(url).toBeInstanceOf(URL); - expect(url.toString()).toBe('http://foo.com/path/to/file.html?a=1&b=2'); - } - /* eslint-enable jest/no-standalone-expect */ + itSerializesEqual('URLSearchParams traced first', { + in() { + const url = new URL('http://foo.com/path/to/file.html?a=1&b=2'); + return {params: url.searchParams, url}; + }, + out: `(()=>{ + const a=new URL("http://foo.com/path/to/file.html?a=1&b=2"); + return{params:a.searchParams,url:a} + })()`, + validate({url, params}) { + expect(url).toBeInstanceOf(URL); + expect(url.toString()).toBe('http://foo.com/path/to/file.html?a=1&b=2'); + expect(params).toBeInstanceOf(URLSearchParams); + expect(params.toString()).toBe('a=1&b=2'); + expect(params).toBe(url.searchParams); + } + }); }); itSerializes.skip('URLSearchParams subclass', {