deep-eql icon indicating copy to clipboard operation
deep-eql copied to clipboard

Cannot reliably compare Ada-based URLs (Node.js v18.17.0 & newer)

Open markandrus opened this issue 2 years ago • 2 comments

Until recently, my team was using Node.js v18.13.0. Our CI system picked up Node.js v18.17.0 and started failing. Specifically, we started failing when using Chai to perform expect(…).to.deep.equal on URLs.

After reading the changelog for v18.17.0, I see that Ada-based URLs were backported to Node.js 18 (see here).

Narrowing down some more, it seems there is a behavior change in Node.js 18.17.0 where Symbol(query) is lazily set on URLs whenever searchParams is accessed (see here). So, if you've never accessed that property of a URL, Symbol(query) does not exist; however, once you have accessed that property, Symbol(query) does exist. This leads to the following breakage (I'm demonstrating with Mocha):

const { expect } = require('chai')

describe('URL', () => {
  it('succeeds', () => {
    const url = new URL('foo://bar')
    expect(url).to.deep.equal(new URL('foo://bar'))
  })

  // The following succeeded in Node.js < v18.17.0.
  it('fails', () => {
    const url = new URL('foo://bar')
    void url.searchParams
    expect(url).to.deep.equal(new URL('foo://bar'))
  })
})

Maybe this is working as expected, but it was an unfortunate bug we hit. We have worked around it by changing our tests to no longer expect(…).to.deep.equal on URLs.

markandrus avatar Jul 19 '23 10:07 markandrus

Is searchParams enumerable? I think a URL will land here: https://github.com/chaijs/deep-eql/blob/b8c99546f7a29d755d7e7e00c10b7a5f651fd15f/index.js#L416 and each key will be checked by getting the property, so I don't know why it would fail.

Oh... unless you're saying the property that houses Symbol(query) is enumerable?

keithamus avatar Jul 19 '23 15:07 keithamus

Hey @keithamus,

Oh... unless you're saying the property that houses Symbol(query) is enumerable?

Yes! Symbol(query) is enumerable, and in Node.js < v18.17.0, Symbol(query) it was always present. Now, in Node.js v18.17.0 (or any version using Ada), Symbol(query) is lazily computed once searchParams is accessed. We can see that here:

Node.js v18.17.0

$ node
Welcome to Node.js v18.17.0.
Type ".help" for more information.
> var url = new URL('foo://bar')
undefined
> Object.getOwnPropertyNames(url)
[]
> Object.getOwnPropertySymbols(url)
[ Symbol(context) ] // ← Symbol(query) is missing
> void url.searchParams
undefined
> Object.getOwnPropertyNames(url)
[]
> Object.getOwnPropertySymbols(url)
[ Symbol(context), Symbol(query) ] // ← Symbol(query) is present and enumerable after accessing searchParams

Node.js v18.13.0

$ node                       
Welcome to Node.js v18.13.0.
Type ".help" for more information.
> var url = new URL('foo://bar')
undefined
> Object.getOwnPropertyNames(url)
[]
> Object.getOwnPropertySymbols(url)
[ Symbol(context), Symbol(query) ] // ← Symbol(query) is present and enumerable
> void url.searchParams
undefined
> Object.getOwnPropertyNames(url)
[]
> Object.getOwnPropertySymbols(url)
[ Symbol(context), Symbol(query) ] // ← Symbol(query) is still present and enumerable

markandrus avatar Jul 21 '23 12:07 markandrus