browserify / browser-resolve

resolve function which support the browser field in package.json
MIT License
102 stars 70 forks source link

updated querystring #16

Closed juliangruber closed 11 years ago

juliangruber commented 11 years ago

Updated querystring to the latest version + my fixes for browsers (see https://github.com/visionmedia/node-querystring/pull/57)

defunctzombie commented 11 years ago

Can we just instead use the node-querystring module like we use http-browserify?

medikoo commented 11 years ago

There's also https://github.com/Gozala/querystring which is a code mirror for Node's querystring

juliangruber commented 11 years ago

that module still needs some shims though

medikoo commented 11 years ago

what shims? what for? I use it without issues in browsers.

juliangruber commented 11 years ago

It uses Object.keys, Array.isArray and more. So it's an issue if you need support for IE<9 without polyfills.

medikoo commented 11 years ago

Yeah, if you need that, you load es5-shim before.

juliangruber commented 11 years ago

Nah, no polyfills for me plz

medikoo commented 11 years ago

@juliangruber without taking advantage of ES5 you're just locking yourself in year 2005.

ghost commented 11 years ago

@shtylman how about merging this now and then when somebody has a version on npm with the same fixes we can depend on that?

juliangruber commented 11 years ago

my PR on qs has already been merged, it just needs to be released.

defunctzombie commented 11 years ago

Since the PR on qs has been merged, I will wait for that to be released (hoping that won't take too long). If someone could ping this PR when that happens I can make the changes to use that module.

On Tue, Apr 23, 2013 at 9:00 AM, Julian Gruber notifications@github.comwrote:

my PR on qs has already been merged, it just needs to be released.

— Reply to this email directly or view it on GitHubhttps://github.com/shtylman/node-browser-resolve/pull/16#issuecomment-16856337 .

juliangruber commented 11 years ago

ping ping ping :)

defunctzombie commented 11 years ago

Looking at the qs module. I don't see it being node.js compatible with the following: http://nodejs.org/api/querystring.html#querystring_querystring_escape

Thoughts?

juliangruber commented 11 years ago

Oh, true....ok then I'll have to make https://github.com/Gozala/querystring run in older browsers...

juliangruber commented 11 years ago

Waiting for https://github.com/Gozala/querystring/pull/4

juliangruber commented 11 years ago

So, Gozala doesn't want to merge my PR, because shims shouldn't be part of the code.

Either we say browserify requires polyfills or we include my fork: https://github.com/juliangruber/querystring/tree/compat.

defunctzombie commented 11 years ago

Since the other projects seem to be going nowhere, I will just merge this. We can deal with other issues as they come up.