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

m.route( pathString, queryObject) creates new param keys #352

Closed
barneycarroll opened this issue Nov 22, 2014 · 7 comments
Closed

m.route( pathString, queryObject) creates new param keys #352

barneycarroll opened this issue Nov 22, 2014 · 7 comments
Labels
Type: Bug For bugs and any other unexpected breakage

Comments

@barneycarroll
Copy link
Member

When assigning strings to keys in the queryObject paramter, my assumption is that an extant param with that key would be replaced, but instead a new key=value pair is appended to the route. In the case where only the last value is read, this is just an issue of ambiguous cruft, but for URI APIs that use multiple key=value pairs to represent list items, this is a functional bug.

@lhorie
Copy link
Member

lhorie commented Nov 24, 2014

Can you provide a jsfiddle illustrating the issue?

@eddyystop
Copy link

Mithril may have a design issue similar to one Node's Express had: http://techblog.netflix.com/2014/11/nodejs-in-flames.html?utm_source=nodeweekly&utm_medium=email

The guts of the article are the 3 paragraphs above "When the Smoke Cleared"

@barneycarroll
Copy link
Member Author

@eddyystop actually Mithril avoids that issue by forcing you to declare routes in one place — you can't add a route node, you can only specify the whole route map in one go.

What I'm talking about is a case where I'm trying to modify a query param with a function that's agnostic of the wider route context:

// location: 'http://example.com?foo=bar'
m.route( m.route(), { foo : 'baz' } );
// location: 'http://example.com?foo=bar&foo=baz'

@barneycarroll
Copy link
Member Author

Here we go. In this reduced case a contrived route hash has a single endpoint, /. The view renders an element which displays the route and, onclick, invokes the full redirection signature of m.route. By my expectation, it should start as /, become /?foo=bar onclick, and stay that way no matter how many times it's clicked. Instead, we quickly end up with a huge string of foo=bar&foo=bar&foo=bar etc.

The snippet, to avoid over-reliance on jsfiddle:

m.route( document.body, '/', {
    '/' : {
        controller : function(){},
        view       : function(){
            return m( 'pre', {
                onclick : function(){
                    m.route( m.route(), {
                        foo : 'bar'
                    } );
                }
            }, m.route() );
        }
    }
} );

@lhorie lhorie added the Type: Bug For bugs and any other unexpected breakage label Nov 24, 2014
@lhorie
Copy link
Member

lhorie commented Nov 24, 2014

Ok yep, this is a bug. Thanks.

@lhorie
Copy link
Member

lhorie commented Nov 26, 2014

Fixed in origin/next

@lhorie lhorie closed this as completed Nov 26, 2014
@barneycarroll
Copy link
Member Author

Nice one Leo!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Type: Bug For bugs and any other unexpected breakage
Projects
None yet
Development

No branches or pull requests

3 participants