Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

removeQuery with value filter doesn't work when the parameter value is not plural / an array / collection. #250

Closed
ryanelian opened this issue Sep 24, 2015 · 6 comments

Comments

@ryanelian
Copy link
Contributor

Example:

var uri = 'http://localhost:2039/product?t=1&q=test';
var uriObj = new URI(uri);
uriObj.removeQuery({t : 1});
return uriObj.href();

Expected returned value:

http://localhost:2039/product?q=test

Actual returned value:

http://localhost:2039/product?t=1&q=test

How I tackled the problem atm:

removeTagQuery: function (uri, tagId) {
    var uriObj = new URI(uri);
    var uriQueries = URI.parseQuery(uriObj.query());

    //hack for https://github.com/medialize/URI.js/issues/250
    //removeQuery with value filter doesn't work when the parameter value is not plural / an array / collection. #250 
    if (Array.isArray(uriQueries.t))
    {
        uriObj.removeQuery({
            t: tagId
        });
    } else
    {
        uriObj.removeQuery('t');
    }

    return uriObj.href();
}
@ryanelian ryanelian changed the title removeQuery with value filter doesn't work with when the parameter value is not plural. removeQuery with value filter doesn't work when the parameter value is not plural / an array / collection. Sep 24, 2015
@rodneyrehm
Copy link
Member

I'm not sure I follow the plural / array / collection thought.

The problem is the type of the value you're trying to remove. To URI.js all query string values are strings.

var uri = "http://localhost:2039/product?t=1&q=test";

URI(uri).removeQuery({t : 1}).toString();
// "http://localhost:2039/product?t=1&q=test"

URI(uri).removeQuery({t : "1"}).toString();
// "http://localhost:2039/product?q=test"

I admit URI.js could've done a better job with casting the value to string itself. To fix that, you'd sinply check for data[name] === String(value) here. Extending the tests is a good idea and please open the PRs against master.

@ryanelian
Copy link
Contributor Author

Very interesting insight.

I originally thought it was array removal problem because URI(uri).removeQuery({t : 1}).toString(); works on http://localhost:2039/product?t=1&t=2&t=3 for instance (Results in http://localhost:2039/product?t=2&t=3), but not when the parameter is not plural.

@rodneyrehm
Copy link
Member

To resolve arrays filterArrayValues() is used, which implictly casts all values to string (because property keys are strings…) - that's why it worked for ?t=1&t=2.

Will you prepare a PR?

@ryanelian
Copy link
Contributor Author

Sorry for the wait! I just got home. Made the PR.

Thanks in advance!

@rodneyrehm
Copy link
Member

fixed by #252

@rodneyrehm
Copy link
Member

released in v1.17.0

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

No branches or pull requests

2 participants