Skip to content

Commit

Permalink
Support URLs v2
Browse files Browse the repository at this point in the history
  • Loading branch information
overlookmotel committed Sep 27, 2023
1 parent f524801 commit 2a20341
Show file tree
Hide file tree
Showing 6 changed files with 86 additions and 54 deletions.
50 changes: 33 additions & 17 deletions lib/serialize/other.js
Original file line number Diff line number Diff line change
Expand Up @@ -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');
Expand All @@ -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
Expand Down Expand Up @@ -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;
Expand All @@ -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');
Expand Down Expand Up @@ -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(
Expand Down
3 changes: 3 additions & 0 deletions lib/serialize/serialize.js
Original file line number Diff line number Diff line change
Expand Up @@ -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};
},

Expand Down
14 changes: 13 additions & 1 deletion lib/serialize/trace.js
Original file line number Diff line number Diff line change
Expand Up @@ -46,6 +46,8 @@ module.exports = {
this.urlSearchParamsPrototypeRecord = this.traceValue(URLSearchParams.prototype, null, null);

this.minusZeroRecord = null;

this.maybeSoloURLSearchParams = [];
},

/**
Expand Down Expand Up @@ -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);
Expand Down Expand Up @@ -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();
}
};

Expand Down
10 changes: 0 additions & 10 deletions package-lock.json

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

1 change: 0 additions & 1 deletion package.json
Original file line number Diff line number Diff line change
Expand Up @@ -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"
},
Expand Down
62 changes: 37 additions & 25 deletions test/other.test.js
Original file line number Diff line number Diff line change
Expand Up @@ -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$/,
Expand Down Expand Up @@ -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) {
Expand Down Expand Up @@ -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) {
Expand All @@ -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', {
Expand Down

0 comments on commit 2a20341

Please sign in to comment.