API does not match Node

[bclinkinbeard]

According to http://nodejs.org/api/querystring.html, the returned object should have escape and unescape methods. Instead, it has encode and decode, with a different API, in that they return objects instead of strings.

[mike-spainhower]

Is this a dupe of the issue in the upstream project Gozala#6 ? This fork is specially made for https://github.com/substack/node-browserify and other projects that need es3 compatibility so any functionality and API issues should be opened with the upstream.

[bclinkinbeard]

Yea, I guess it is.

[dy]

Hello, guys @bclinkinbeard @mike-spainhower
We decided to get rid of jquery in our project, and we need node-jquery-param module to replace $.param.
Unfortunately, we've found that current browserify doesn't provide correct querystring module, particularly, it doesn't have .escape method, which results in errors.
It happens because of incompatibility of querystring-es3 API with node v0.10+ behavior.
That is really unfortunate.
Not sure where I have to place this issue, but I really hope on your aid.

[mike-spainhower]

Ok, since the maintainer of the upstream is being actively unhelpful, and browserify users (I am one also) seem to need this compatibility, I am willing to fully depart from the upstream and add this functionality in this repo.

@substack before I do this, would adding escape and unescape methods for node compat have any negative consequences for browserify, or is there any other consideration? Thanks.

[troggy]

What's the status of the issue? I'm stuck with using browserified request library which relies on a presence of unescape method

[SpainTrain]

Pinging @substack again on question above. I would not want to add this until @substack weighs in on any potential negative effects.

In the meantime, if anyone affected would like to open a PR for this, feel free.

[mathiasvr]

Hi, I extracted the querystring implementation from the node.js core at https://github.com/mathiasvr/querystring. It seems to work fine in browsers, passing the tests down to IE9. I could make a PR, but i guess we might as well wait and see what substack thinks 😉

[SpainTrain]

That's great @mathiasvr, thanks for the offer. Hopefully we can go that route.

[Beirdo]

This is crippling my use of the aws4 npm module in browserify as well. Is there any progress on this?

[mhart]

@Beirdo there's an example of how to use aws4 with browserify here: https://github.com/mhart/aws4/tree/master/browser

[Beirdo]

Thank you. I have used this as a viable workaround, but really, the upstream querystring modules used by browserify need some work :)

My requests to the API Gateway from my phone application are now AWSv4 signed and all is good.

[spalger]

I have started using an alias to the querystring-browser module that @mathiasvr created:

resolve: {
  alias: {
    querystring: 'querystring-browser'
  }
}

[dy]

Hi guys!
I am surprised how long this issue hasn’t been resolved.
querystring.stringify actually also has options as a last argument, which takes encodeURIComponent param. That might be handy to do github-like queries &utf8=✔ etc. With current implementation it is impossible to redefine encodeURIComponent, which is sad.
What are possible ways we can contribute to make querystring really compatible with node?

[SpainTrain]

how about this:

1. a well tested PR for this issue will be merged
2. the new version will be released behind a major bump
3. a PR is opened against browserify to use the new major version

I no longer use browserify for any large projects, so would love volunteers to open the PR and test it

[SpainTrain]

pinging @mathiasvr about PR

[mathiasvr]

@SpainTrain Nodes querystring implementation is updated quite often and is relying more and more on ES6 features. Because of this I've been thinking about using babel to generate the future versions of my module. I don't know what an obvious PR would be for this right now? Do you want the module to be designed for the browser, or is a babel version of the core implementation fine? Anyway I will probably have to get back to this, until I update my module. Otherwise I hope someone else can weigh in!

[SpainTrain]

Frankly the requirements come from the dependents - https://www.npmjs.com/package/querystring-es3. Originally this was just a one-off module for browserify, but now has 21 dependents.

To actually answer the question: I suspect that a babel-compiled version of node.js core is appropriate. The question is which version does it come from, and I think the latest LTS version probably makes sense. Thoughts?

[balu-]

would the change from Trott (Trott@46c1e33) be acceptable to solve this issue?

[SpainTrain]

Sorry for the silence on this very old issue. I have updated this lib at https://github.com/SpainTrain/querystring-es3 using the node 6.10.2 implementation of (and tests for) querystring.

I will shortly be publishing a pre-release behind a major bump for testing.

Apologies again on how absurdly long this issue has been open.

Please feel free to review https://github.com/SpainTrain/querystring-es3 and report issues

[Beirdo]

Thanks. I'll take a look.

…

On Sun, Apr 3, 2016 at 7:40 PM, Michael Hart ***@***.***> wrote: @Beirdo <https://github.com/Beirdo> there's an example of how to use aws4 with browserify here: https://github.com/mhart/aws4/tree/master/browser — You are receiving this because you were mentioned. Reply to this email directly or view it on GitHub <#4 (comment)>