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

Default can.route.ready to false #298

Closed
daffl opened this issue Mar 5, 2013 · 16 comments
Closed

Default can.route.ready to false #298

daffl opened this issue Mar 5, 2013 · 16 comments
Milestone

Comments

@daffl
Copy link
Contributor

daffl commented Mar 5, 2013

For 1.2 we should default can.route.ready to always be false and let the user initialize routing in any case.

When to use can.route.ready and when not seems to cause quite a bit of confusion and in almost every case you need it anyway. Plus we can get rid of the rather hacky document ready detection (which had to be added because the document ready handler in libraries other than jQuery doesn't get called if the document is ready by the time it is attached).

@justinbmeyer
Copy link
Contributor

I worry about basic apps. Those using can/control/route.

On Mar 5, 2013, at 10:08 AM, David Luecke notifications@github.com wrote:

For 1.2 we should default can.route.ready to always be false and let the user initialize routing in any case.

When to use can.route.ready and when not seems to cause quite a bit of confusion and in almost every case you need it anyway. Plus we can get rid of the rather hacky document ready detection (which had to be added because the document ready handler in libraries other than jQuery doesn't get called if the document is ready by the time it is attached).


Reply to this email directly or view it on GitHub.

@jeffreytgilbert
Copy link

I'm having issues with this. CanJS is rewriting a route and escaping characters in it before i can stop it from running and doing so. It's extremely confusing because I haven't even told CanJS about any routes. In fact, removing all routing declarations and just loading CanJS on a page will alter my routes.

Reasons it shouldn't be run by default:

  • No routing may even be loaded, so it's doing things it's not asked to be doing, which is contrary to expectations
  • Third party routing libraries will be interfered with, thereby breaking "library-er" conventions of CanJS
  • Deferring the route handling to a point where the asynchronous modules have a chance to load and respond in their own order just makes more sense
  • Adding a ready call at the end of a route declaration is sufficiently easy to understand for basic app builders

@justinbmeyer
Copy link
Contributor

You can make a download without can/route. That should solve the issues you're having.

@jeffreytgilbert
Copy link

If I don't use the complicated routes with parameters, it doesn't happen, but then i also can't use those features. I'm using the AMD / RequireJS version, not the downloaded bundle.

@jeffreytgilbert
Copy link

BTW, this bug can be seen in the TODO MVC example as well as my application. Here's a url to showcase what is happening.

http://addyosmani.github.com/todomvc/labs/dependency-examples/canjs_require/#!active/andthis&butthis=thisval

Watch the / after active to see what's happening. It gets encoded, and if it were a route, it would no longer match the route definition. You can actually see this by adding a number of characters like so:

http://addyosmani.github.com/todomvc/labs/dependency-examples/canjs_require/#!active;andthis=this&butthis=thisval

The semicolon will be encoded as well as the following = sign.

@justinbmeyer
Copy link
Contributor

If I don't use the complicated routes with parameters, it doesn't happen, but then i also can't use those features. I'm using the AMD / RequireJS version, not the downloaded bundle.

What do you mean?

BTW, this bug ...

This issue is about can.route.ready being set to false. I'm not sure exactly what you are talking about. However, if you think there's another bug, can you create another issue? Also, can you carefully describe the bug? Ideally someone should be able to read the comment and know immediately what you are talking about. Thanks.

EDIT:

After spending some time trying to understand what you meant, I'm not even sure why #!active/andthis&butthis=thisval would be a route? Nothing in the app set it that way. There's only one route:

https://github.com/addyosmani/todomvc/blob/gh-pages/architecture-examples/canjs/js/app.js#L6

can.route(':filter');

With just this one route, I would expect a hash like #!foo/bar to get converted to #!foo%2Fbar so can.route.attr() would return:

{filter: "foo/bar", route: ":filter"}

I think you might be misunderstanding the idea behind can.route. can.route isn't as much about matching a hash as it is keeping state.

say, for example there were two routes:

can.route(':filter');
can.route(':filter/:something');

And the user set can.route w/ .attr() like:

can.route.attr("filter","foo/bar")

The hash would have to be set to #!foo%2Fbar. If it was set to #!foo/bar, the data would be:

{filter: "foo", something: "bar", route: ":filter/:something"}

Basically, can.route does whatever it has to so the has represents one unique state.

@jeffreytgilbert
Copy link

I've added another issue here per your request:

#330

@jeffreytgilbert
Copy link

You're absolutely right about that not being a route that the TODO MVC application is expecting. It wouldn't matter even if it were. CanJS shouldn't be manipulating those hashes. If it were actually expecting those routes, it wouldn't change the behavior of the page. The encoding would still happen and the routes would still break.

@justinbmeyer
Copy link
Contributor

If you are using the amd version, don't require 'can' directly, create a custom 'mycan' that requires only the modules you want.

Also, you might be able to simply call

can.route.ready(false)

and never call .ready(true). As you are using requirejs, to use the plugin that prevent's jQuery's ready event from firing until all scripts have loaded.

@justinbmeyer
Copy link
Contributor

If it were actually expecting those routes .... the encoding would still happen and the routes would still break.

That's not right. Check out the example in #330.

I think the best solution for @jeffreytgilbert is simply not to load can.route if you aren't using it. Don't waste time loading code you aren't using.

If we should call force a call to can.route.ready() when someone has included the plugin (indicating desire of its features) is another matter ...

@jeffreytgilbert
Copy link

I'm currently using the routing for simple routes. Complex ones i'll have another pass at a few months down the road once i've finished everything mission critical on this project.

On Mar 21, 2013, at 3:08 PM, Justin Meyer notifications@github.com wrote:

If it were actually expecting those routes .... the encoding would still happen and the routes would still break.

That's not right. Check out the example in #330.

I think the best solution for @jeffreytgilbert is simply not to load can.route if you aren't using it. Don't waste time loading code you aren't using.

If we should call force a call to can.route.ready() when someone has included the plugin (indicating desire of its features) is another matter ...


Reply to this email directly or view it on GitHub.

@amcdnl
Copy link
Contributor

amcdnl commented Apr 8, 2013

+1 I think we should start this off as false too.

Its a pretty common thing to need to do, shouldn't have to have the users doing it.

@justinbmeyer
Copy link
Contributor

Brought here from #475. Again, my big hesitation is that we will have to tell people they MUST call can.route.ready() to even use routing in the first place. The getting started guide will have can.route.ready().

@ccummings
Copy link
Contributor

The confusion/frustration from the automatic ready is likely related to #473 where ready is called once (before user has a chance to delay) and then again when jQuery's ready event is fired.

Backbone and Ember both require an explicit call to start routing using Backbone.history.start() and Ember.Application.create(...) respectively.

In Ember's case it will start routing on DOMReady, but you can control that using deferReadiness and advanceReadiness. Angular does something similar except it seems much more tricky to delay route initialization.

I'd be fine with automatic route.ready on DOMReady if we added a ready event to all libraries which gives the user a chance at delaying the route initialization.

@daffl
Copy link
Contributor Author

daffl commented Sep 12, 2013

I found it way harder having to explain (and document) when to use can.route.ready and why it works in one case but doesn't in another. This comes up in IRC and Stackoverflow all the time and it just doesn't seem to work how many people (including myself) expect it to.

Plus personally when I do things like:

var Router = can.Control.extend({
    ':type route': function() {
        // do something
    }
});

new Router();

I always want my route to fire even when the page is refreshed and the router is initialized and for that there is no way around having to call can.route.ready (twice).

@justinbmeyer
Copy link
Contributor

With 520f1b9, you MUST call can.route.ready. This will be part of 1.2.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

5 participants