defunctzombie / node-url

node.js core url module as a module
MIT License
375 stars 96 forks source link

The Principle of Least Astonishment is broken #29

Closed rtodea closed 3 years ago

rtodea commented 7 years ago

Consider the following scenario:

const url = require("url");
const someUrl = 'http://google.com?first=this&second=that';
const urlModel = url.parse(someUrl, true);

// change the query params
delete urlModel.query.first;
urlModel.query.third='some';

console.log(urlModel.query);

// expect the new URL to have the query params changed
console.log(url.format(urlModel));
// expected: http://google.com/?second=that&third=some
// actual: http://google.com?first=this&second=that

Instead if I modify the .search everything works as expected

const url = require("url");
const someUrl = 'http://google.com?first=this&second=that';
const urlModel = url.parse(someUrl, true);

// change the search string

urlModel.search = 'second=that&third=some';

// expect the new URL to have the query params changed
console.log(url.format(urlModel));
// http://google.com/?second=that&third=some

I have not found this behavior documented and I think it breaks POLA.

We have 2 options:

  1. document this
  2. change the behavior
lanre-ade commented 7 years ago

👍 I thought I was going crazy. Glad to know someone else has come across this as well

sheepsteak commented 6 years ago

Looks like this is the behaviour in Node too and according to the docs for url.format search takes precedence over query.

ljharb commented 3 years ago

Whatever node does, this module should do. I'd suggest filing a bug on node itself if you find its behavior surprising.