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

Please add trailing slash support for can.route.pushstate #588

Closed
stevenvachon opened this issue Dec 3, 2013 · 16 comments
Closed

Please add trailing slash support for can.route.pushstate #588

stevenvachon opened this issue Dec 3, 2013 · 16 comments
Milestone

Comments

@stevenvachon
Copy link
Contributor

No description provided.

@justinbmeyer
Copy link
Contributor

Can you add a description / example? Why is trailing slash support needed? If you wanted to match everything under foo/, you can set the root to foo.

@stevenvachon
Copy link
Contributor Author

Trailing slashes are HTTP spec for directories. It's common practice on marketing-type websites (blogs, etc) to simulate folders instead of resources. Would be a good idea to carry this mentality over to apps as they too rely on marketing tactics.

If we have an app at / and we want to support those with JavaScript disabled, we'd use something like NodeJS to serve them a generated HTML file using /index.html (and CanJS production files) when they try to access routes like /view1/sub-view/ that don't actually have a corresponding file/directory structure. When copying URLs between the JS and non-JS versions of the app, they should be able to work without automatic HTTPD redirects.

@daffl
Copy link
Contributor

daffl commented Dec 3, 2013

I can't find a reference for that use of trailing slashes in the HTTP spec. Do you have a link?
All that I can see is that a trailing slash should denote a folder on the webserver which doesn't really apply to pushstate. Escaping route/ to route%2F is a little weird but if you find this happened you could still just redirect to route.

@stevenvachon
Copy link
Contributor Author

I edited my previous comment a few times to make it more accurate. Please check it out.

How does a virtual folder not apply to pushstate? We're already pointing to them, only difference is we're currently forced to end with a resource.

@justinbmeyer
Copy link
Contributor

I still don't really understand. Can you give me a code example or fiddle? Afaik, trailing slashes already work.

@stevenvachon
Copy link
Contributor Author

This link ends up going to /test%2F:

can.route.bindings.pushstate.root = "/";
can.route(":section");
can.route.ready();
<a href="test/">link</a>

This ends up going to /test//:

can.route.bindings.pushstate.root = "/";
can.route(":section/:sub/");
can.route.ready();
can.route.attr({section:"test",sub:""});

@justinbmeyer
Copy link
Contributor

Could you change that to:

can.route.bindings.pushstate.root = "/";
can.route(":section/");
can.route.ready();

As far as the other example, that is what I would expect. You can probably fix that like:

can.route.bindings.pushstate.root = "/";
can.route(":section/",{
  sub: ""
});
can.route(":section/:sub/");
can.route.ready();
can.route.attr({section:"test",sub:""});

@stevenvachon
Copy link
Contributor Author

Changing to ":section/" resulted in my href="test/" link not dispatching a route event. A link with href="test" dispatches the event but then there's no trailing slash in the address bar.

@justinbmeyer
Copy link
Contributor

Ok, so that's a bug. I'll try to fix this asap.

@stevenvachon
Copy link
Contributor Author

Cool, thank you

@justinbmeyer
Copy link
Contributor

Ok, I created a test that looks like:

    test("trailing slashes (#588)", function(){
        can.route.routes = {};
        can.route.bindings.pushstate.root = "/";

        can.route(":section/");
        res = can.route.param({section: "test"});

        equal(res, "test/");

        var obj = can.route.deparam("foo/");
        deepEqual(obj, {
            section : "foo",
            route: ":section/"
        });

    });

And it passes. I'm a bit confused. How are you creating the link?

@justinbmeyer
Copy link
Contributor

If I do:

<script>
    steal("can/route/pushstate", function(){
        can.route.bindings.pushstate.root = "/";
        can.route(":section/");
        can.route.ready();

        $(document.body).append("<br/>",can.route.link("foo",{
            section: "foo"
        }))

        can.route.bind("section", function(ev, newVal){
            console.log("section updated to",newVal)
        })
    })

</script>
<a href="/test/">test</a>

This calls back the section event handler. You'll notice that the link has to include the root's / like: <a href="/test/">test</a>. To make this easier, use can.route.link.

@justinbmeyer
Copy link
Contributor

I'm going to close unless as there does not appear to be a bug. Please comment if I'm missing something.

@stevenvachon
Copy link
Contributor Author

can.route.bindings.pushstate.root = "/";
can.route(":section/");
can.route.ready();

can.route.attr({section: "test"});

goes to /test, not /test/ on my machine

and

can.route.bindings.pushstate.root = "/";
can.route(":section/");
can.route.ready();

console.log( can.route.url({section: "test"}) );

gives me /test, not /test/

am I doing something incorrectly?

@justinbmeyer
Copy link
Contributor

@stevenvachon I'm not sure. Let chat on skype tomorrow: justinbmeyer.

@justinbmeyer justinbmeyer reopened this Dec 19, 2013
@justinbmeyer
Copy link
Contributor

This is confirmed working with @stevenvachon

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

3 participants