Skip to content

breaking(): remove legacy code and introduce browser entry#301

Open
patricksevat wants to merge 3 commits into
developfrom
breaking/v4
Open

breaking(): remove legacy code and introduce browser entry#301
patricksevat wants to merge 3 commits into
developfrom
breaking/v4

Conversation

@patricksevat

Copy link
Copy Markdown
Collaborator

The current implementation has several imports which are not browser friendly:

  1. require('url') in encoder/index
  2. require('querystring') in index
  3. require('url') in legacy

This PR fixes those by providing a browser entrypoint which is mostly based on the browser's URL api: https://developer.mozilla.org/docs/Web/API/URL while trying to keep the api and results the same.

That said, there are some breaking changes and behavioral differences:

BREAKING

Behavorial changes, only applicable to the BROWSER ENTRYPOINT

  • .encodeQueryString with multi values behaves differently:
encodeQueryString({
            q1: ['𝌆й', '你ス'],
            q2: ''
        })
  • NodeJS: encode to the equivalent of 'q1=𝌆й&q1=你ス&q2=' (note the double q1 param)
  • Browser: encode to the equivalent of 'q1=𝌆й,你ス&q2=' (single, comma-separated q1 param)
  • invalid URLs are dealt with differently: toNodeUrl('xn--iñvalid.com')
    • NodeJS returns host: 'xn--iñvalid.com',
    • Browser returns: host: '-xn--xn--ivalid-x9a.com'

(typeof window === 'undefined' ? it : it.skip)('should return input value on invalid domain', function () {
expect(encoder.encodeHost('xn:')).to.equal('xn:');
expect(encoder.encodeHost('example#com')).to.equal('example#com');
// expect(encoder.encodeHost('example#com')).to.equal('example#com');

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

In newer Node versions (I used Node 22), this breaks

'HTTP://example.com',
'http://xn--48jwgn17gdel797d.com',
'http://xn--iñvalid.com',
// 'http://xn--iñvalid.com',

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

In newer Node versions, this breaks

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant