unlock unlimited code runs for $79, limited time →
NOSSLETTER.techLIVE
back to OSSpedia
api-designrefactoringtesting-strategy

url: align URLPatternInit dictionary conversion with WebIDL

Project: nodejs/node | Language: JavaScript/C++ | Concepts: api-design, refactoring, testing-strategy | Difficulty: mid
Link: https://github.com/nodejs/node/pull/65498

The problem

The URLPatternInit dictionary in Node.js did not match the WebIDL and WHATWG specifications in two key ways:

  • The dictionary’s properties were not accessed in the required lexicographical order, which can break code that relies on traps or getters (like Proxies).
  • When converting to JavaScript objects, absent dictionary members were included as properties with undefined values, instead of being omitted entirely as required.

Why it's non-trivial

A naive fix might reorder the property accesses in one place but miss other code paths, or simply set properties to undefined instead of omitting them. Both behaviors can cause subtle bugs, especially when users depend on exact property enumeration order or rely on the absence/presence of properties for correctness (e.g., for feature detection or when using Proxies).

The approach

1. Reordering property access to lexicographical order

The code changes the order in which dictionary members are defined and accessed to match the WebIDL spec:

static constexpr std::string_view namesVec[] = {
-    "protocol",
-    "username",
-    "password",
+    "baseURL",
+    "hash",
     "hostname",
-    "port",
+    "password",
     "pathname",
+    "port",
+    "protocol",
     "search",
-    "hash",
-    "baseURL",
+    "username",
};

Annotation:
This sets the canonical, lexicographical order for property access, ensuring that any traps (like those on a Proxy) will observe property accesses in the right sequence.

2. Matching the order for value assignment

The code updates the order in which values are assigned to match the new property order:

-  if (!trySet(init.protocol) || !trySet(init.username) ||
-      !trySet(init.password) || !trySet(init.hostname) || !trySet(init.port) ||
-      !trySet(init.pathname) || !trySet(init.search) || !trySet(init.hash) ||
-      !trySet(init.base_url)) {
+  if (!trySet(init.base_url) || !trySet(init.hash) || !trySet(init.hostname) ||
+      !trySet(init.password) || !trySet(init.pathname) || !trySet(init.port) ||
+      !trySet(init.protocol) || !trySet(init.search) ||
+      !trySet(init.username)) {
    return {};
  }

Annotation:
The order of trySet calls now matches the property order, ensuring values are filled in the correct slots.

3. Omitting absent members

Instead of always filling a fixed array with Undefined, the code now only assigns present values, and uses the template’s NewInstance to create a sparse object:

-  MaybeLocal<Value> values[] = {
-      Undefined(isolate),  // protocol
-      Undefined(isolate),  // username
-      Undefined(isolate),  // password
-      Undefined(isolate),  // hostname
-      Undefined(isolate),  // port
-      Undefined(isolate),  // pathname
-      Undefined(isolate),  // search
-      Undefined(isolate),  // hash
-      Undefined(isolate),  // baseURL
-  };
-
+  MaybeLocal<Value> values[9];

and

-  return NewDictionaryInstance(env->context(), tmpl, values);
+  return tmpl->NewInstance(context, values);

Annotation:
By using the template’s NewInstance with only the present values, absent properties are omitted from the resulting object, matching WebIDL’s behavior.

4. Regression tests for order and property omission

The PR adds tests to verify both the property access order and the omission of absent members:

{
  const accessed = [];
  const expected = [
    'baseURL',
    'hash',
    'hostname',
    'password',
    'pathname',
    'port',
    'protocol',
    'search',
    'username',
  ];
  const init = new Proxy({}, {
    get(target, name, receiver) {
      accessed.push(name);
      return Reflect.get(target, name, receiver);
    },
  });

  new URLPattern(init);
  assert.deepStrictEqual(accessed, expected);
}

Annotation:
This test uses a Proxy to record the order of property accesses, asserting they match the spec.

{
  const input = new URLPattern({ pathname: '/x' })
    .exec({
      protocol: 'https',
      pathname: '/x',
      username: undefined,
    }).inputs[0];

  assert.deepStrictEqual(Object.keys(input), ['pathname', 'protocol']);
  assert.strictEqual('username' in input, false);
  assert.deepStrictEqual({ ...input }, {
    pathname: '/x',
    protocol: 'https',
  });
}

Annotation:
This test checks that absent members (like username) are not present as own properties, even if explicitly set to undefined.

From the review thread

No notable review discussion on this PR.

Takeaways

  • Always match property enumeration and access order to the specification, especially when user code may trap or depend on access order (e.g., via Proxies).
  • When converting dictionaries or objects to JavaScript, omit absent members rather than setting them to undefined to avoid leaking implementation details and to match spec semantics.
  • Regression tests should cover both property order and presence/absence of properties, not just values, to catch subtle spec mismatches.