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

Before Filter / After Filter on Backbone.Controller #299

Closed
wants to merge 1 commit into
base: master
from

Conversation

Projects
None yet
10 participants
@datapimp

datapimp commented Mar 30, 2011

added before filter, after filter, tests, and documentation for Backbone.Controller

@elliottneilclark

This comment has been minimized.

Show comment
Hide comment
@elliottneilclark

elliottneilclark Apr 6, 2011

That commit is not valid javascript. So I fixed the issue on my repo elliottneilclark@c2f1867f3a0e19f54f47dfe8bea9a448e0cc8db2

elliottneilclark commented Apr 6, 2011

That commit is not valid javascript. So I fixed the issue on my repo elliottneilclark@c2f1867f3a0e19f54f47dfe8bea9a448e0cc8db2

@jashkenas

This comment has been minimized.

Show comment
Hide comment
@jashkenas

jashkenas Apr 18, 2011

Owner

This looks great for your fork of Backbone, but I think it's too edge-case to be broadly applicable to everyone.

Owner

jashkenas commented Apr 18, 2011

This looks great for your fork of Backbone, but I think it's too edge-case to be broadly applicable to everyone.

@jashkenas jashkenas closed this Apr 18, 2011

@ghost

This comment has been minimized.

Show comment
Hide comment
@ghost

ghost Jul 3, 2011

I couldn't disagree more. Before/after filters are a commonly exercised feature of most frameworks, and are a great way to handle login states.

ghost commented Jul 3, 2011

I couldn't disagree more. Before/after filters are a commonly exercised feature of most frameworks, and are a great way to handle login states.

@siong1987

This comment has been minimized.

Show comment
Hide comment
@siong1987

siong1987 Jul 10, 2011

Can someone reopen this ticket? This seems like a really important feature for most of the frameworks.

siong1987 commented Jul 10, 2011

Can someone reopen this ticket? This seems like a really important feature for most of the frameworks.

@jashkenas

This comment has been minimized.

Show comment
Hide comment
@jashkenas

jashkenas Jul 10, 2011

Owner

Not yet -- but I'd be glad to discuss it. "Most frameworks" aren't in-browser, stateful libraries ... and in JavaScript, and function can have a "before" or "after" filter by simply wrapping it in another function:

http://documentcloud.github.com/underscore/#wrap

Do you have any specific, real-world use cases where before filters would be helpful in Backbone?

Owner

jashkenas commented Jul 10, 2011

Not yet -- but I'd be glad to discuss it. "Most frameworks" aren't in-browser, stateful libraries ... and in JavaScript, and function can have a "before" or "after" filter by simply wrapping it in another function:

http://documentcloud.github.com/underscore/#wrap

Do you have any specific, real-world use cases where before filters would be helpful in Backbone?

@siong1987

This comment has been minimized.

Show comment
Hide comment
@siong1987

siong1987 Jul 10, 2011

I actually pull it out as a plugin now: https://github.com/FLOChip/backbone_router_filter

It can be more useful if it is similar as how Rails's before_filter and after_filter work where we can set which methods that the filter methods actually applies on.

But, I guess it might be better to use it as a plugin for now.

siong1987 commented Jul 10, 2011

I actually pull it out as a plugin now: https://github.com/FLOChip/backbone_router_filter

It can be more useful if it is similar as how Rails's before_filter and after_filter work where we can set which methods that the filter methods actually applies on.

But, I guess it might be better to use it as a plugin for now.

@datapimp

This comment has been minimized.

Show comment
Hide comment
@datapimp

datapimp Jul 10, 2011

Having built a pretty huge app in which this feature played a role, I agree that it shouldn't be part of the framework. It is easy enough to extend backbone on your own when you need this, especially with underscore

datapimp commented Jul 10, 2011

Having built a pretty huge app in which this feature played a role, I agree that it shouldn't be part of the framework. It is easy enough to extend backbone on your own when you need this, especially with underscore

@ghost

This comment has been minimized.

Show comment
Hide comment
@ghost

ghost Jul 11, 2011

Managing OAuth state the way that Sammy does would be one specific use case.

ghost commented Jul 11, 2011

Managing OAuth state the way that Sammy does would be one specific use case.

@elliottneilclark

This comment has been minimized.

Show comment
Hide comment
@elliottneilclark

elliottneilclark Jul 11, 2011

I use it to clean up listeners and to kill requests generated by one view that are no longer being displayed.

elliottneilclark commented Jul 11, 2011

I use it to clean up listeners and to kill requests generated by one view that are no longer being displayed.

@ghost

This comment has been minimized.

Show comment
Hide comment
@ghost

ghost Jul 13, 2011

And if _.wrap is the approved means of approaching this problem, how exactly should it be done? If I call it in the Router object's initialize, it doesn't actually do anything as Backbone seems to have already picked up the reference to the original, unwrapped function.

ghost commented Jul 13, 2011

And if _.wrap is the approved means of approaching this problem, how exactly should it be done? If I call it in the Router object's initialize, it doesn't actually do anything as Backbone seems to have already picked up the reference to the original, unwrapped function.

@rxgx

This comment has been minimized.

Show comment
Hide comment
@rxgx

rxgx Aug 19, 2011

Do before and after filters make sense in JavaScript? It's not like we have an notion of a class (let alone abstract and base to extend or implement) to apply annotations before and after a given constructor is called. I think the _.wrap method is for the best, overall, since the trend leads away from constructor functions implemented by car = new Vehicle() to car = Object.create(vehicle).

rxgx commented Aug 19, 2011

Do before and after filters make sense in JavaScript? It's not like we have an notion of a class (let alone abstract and base to extend or implement) to apply annotations before and after a given constructor is called. I think the _.wrap method is for the best, overall, since the trend leads away from constructor functions implemented by car = new Vehicle() to car = Object.create(vehicle).

@ghost

This comment has been minimized.

Show comment
Hide comment
@ghost

ghost Aug 21, 2011

Whether or not it makes sense in JavaScript, Backbone is providing a notion of a class, and using the framework requires that you inherit from these notions, so that seems like kind of a nonsensical argument.

Somebody show me where and when _.wrap() will actually work in a Backbone Router definition and I'll gladly do it. Nothing I've tried has ever worked.

ghost commented Aug 21, 2011

Whether or not it makes sense in JavaScript, Backbone is providing a notion of a class, and using the framework requires that you inherit from these notions, so that seems like kind of a nonsensical argument.

Somebody show me where and when _.wrap() will actually work in a Backbone Router definition and I'll gladly do it. Nothing I've tried has ever worked.

@shesek

This comment has been minimized.

Show comment
Hide comment
@shesek

shesek Aug 21, 2011

Contributor

@nsdpt, Set a wrapper function as var wrapper = function(callback) { /* ... before ... */ callback.apply(this, Array.prototype.slice.call(arguments,1)); /* ... after ... */ } and use to _.wrap(router_callback, wrapper) as your router callbacks. You will have to call Backbone.Router#route() manually from initialize() for that, as you can't use plain callbacks in routes property - only method names as a string.

If you do want to use the routes object, you could also execute something like _.each(['foo', 'bar', 'baz', 'qux'], function(method){ router[method] = _.wrap(wrapper, method); }), where foo/bar/baz/qux are the router methods you want to apply before/after on, router is your router object and wrapper is the function above, allowing you to use the regular 'regex': 'method_name' in the routes property.

Alternatively, you can work with the route:... event as an alternative to after(), and the beforeroute:... event from #494 as an alternative forbefore().

Contributor

shesek commented Aug 21, 2011

@nsdpt, Set a wrapper function as var wrapper = function(callback) { /* ... before ... */ callback.apply(this, Array.prototype.slice.call(arguments,1)); /* ... after ... */ } and use to _.wrap(router_callback, wrapper) as your router callbacks. You will have to call Backbone.Router#route() manually from initialize() for that, as you can't use plain callbacks in routes property - only method names as a string.

If you do want to use the routes object, you could also execute something like _.each(['foo', 'bar', 'baz', 'qux'], function(method){ router[method] = _.wrap(wrapper, method); }), where foo/bar/baz/qux are the router methods you want to apply before/after on, router is your router object and wrapper is the function above, allowing you to use the regular 'regex': 'method_name' in the routes property.

Alternatively, you can work with the route:... event as an alternative to after(), and the beforeroute:... event from #494 as an alternative forbefore().

@yfeldblum

This comment has been minimized.

Show comment
Hide comment
@yfeldblum

yfeldblum Aug 31, 2011

Aspects (such as before filters and after filters) would be highly useful.

yfeldblum commented Aug 31, 2011

Aspects (such as before filters and after filters) would be highly useful.

@angelo0000

This comment has been minimized.

Show comment
Hide comment
@angelo0000

angelo0000 Oct 26, 2011

For our backbone app we use a small filter module I wrote: https://github.com/angelo0000/backbone_filters. It allows for filters based on routes instead of just a single before/after per Router/Controller. It acts more like a rails before filter in that if it returns false, it stops the execution chain.

angelo0000 commented Oct 26, 2011

For our backbone app we use a small filter module I wrote: https://github.com/angelo0000/backbone_filters. It allows for filters based on routes instead of just a single before/after per Router/Controller. It acts more like a rails before filter in that if it returns false, it stops the execution chain.

@ghost

This comment has been minimized.

Show comment
Hide comment
@ghost

ghost Oct 26, 2011

Sounds perfect, but supporting only versions below 0.5 is probably a dealbreaker for most.

ghost commented Oct 26, 2011

Sounds perfect, but supporting only versions below 0.5 is probably a dealbreaker for most.

@angelo0000

This comment has been minimized.

Show comment
Hide comment
@angelo0000

angelo0000 Oct 26, 2011

Agreed, we have not upgraded our version of backbone in our app so I added that disclaimer because I'm sure it would need changes to support the latest... maybe I'll get it up to par with the latest version of backbone.

angelo0000 commented Oct 26, 2011

Agreed, we have not upgraded our version of backbone in our app so I added that disclaimer because I'm sure it would need changes to support the latest... maybe I'll get it up to par with the latest version of backbone.

@posabsolute

This comment has been minimized.

Show comment
Hide comment
@posabsolute

posabsolute May 30, 2012

+1 on this, (even if it is old), before filters are very attractive to handle routes states. For example, you want to fade out your old view on a mobile web app. with a before filter you can add this once instead of having it at the beginning of each routes.

posabsolute commented May 30, 2012

+1 on this, (even if it is old), before filters are very attractive to handle routes states. For example, you want to fade out your old view on a mobile web app. with a before filter you can add this once instead of having it at the beginning of each routes.

@datapimp

This comment has been minimized.

Show comment
Hide comment
@datapimp

datapimp May 31, 2012

so, I actually disagree with having this handled by the router. I initially made this feature request at a time when I didn't really understand the framework at the level I do now.

Luca.Router = Backbone.Router.extend
  routes:
    "" : "default"

  initialize: (@options)->
    _.extend @, @options

    @routeHandlers = _( @routes ).values()

    # when a route handler is fired, the route:route_name event is triggered by the router
    # unfortunately this doesn't apply to calls to @navigate() so we override Backbone.Router.navigate
    # and trigger an event separately.
    _( @routeHandlers ).each (route_id) =>
      @bind "route:#{ route_id }", ()=>
        @trigger.apply @, ["change:navigation", route_id  ].concat( _( arguments ).flatten() )

  #### Router Functions

  # Intercept calls to Backbone.Router.navigate so that we can at least
  # build a path from the route, even if we don't trigger the route handler
  navigate: (route, triggerRoute=false)->
    Backbone.Router.prototype.navigate.apply @, arguments
    @buildPathFrom( Backbone.history.getFragment() )

  # given a url fragment, construct an argument chain similar to what would be
  # emitted from a normal route:#{ name } event that gets triggered
  # when a route is actually fired.  This is used to trap route changes that happen
  # through calls to @navigate()
  buildPathFrom: (matchedRoute)->
    _(@routes).each (route_id, route)=>
      regex = @_routeToRegExp(route)
      if regex.test(matchedRoute)
        args = @_extractParameters(regex, matchedRoute)
        @trigger.apply @, ["change:navigation", route_id].concat( args )

What I do now is customize my router so that it emits route events every time somebody makes a call to router.navigate ( regardless if the trigger route param is set to true. ) Which, IMO, is an improvement over the stock backbone behavior

I almost always have a single application object ( a Luca.Viewport ) which all other views can bind to, or access to get global state. The application listens to change:navigation events, and then updates its internal state machine, so all interested parties can respond appropriately.

This eliminates the needs for before / after filters in the router, and moves the concern to where I think it is more appropriate, which is in your views.

datapimp commented May 31, 2012

so, I actually disagree with having this handled by the router. I initially made this feature request at a time when I didn't really understand the framework at the level I do now.

Luca.Router = Backbone.Router.extend
  routes:
    "" : "default"

  initialize: (@options)->
    _.extend @, @options

    @routeHandlers = _( @routes ).values()

    # when a route handler is fired, the route:route_name event is triggered by the router
    # unfortunately this doesn't apply to calls to @navigate() so we override Backbone.Router.navigate
    # and trigger an event separately.
    _( @routeHandlers ).each (route_id) =>
      @bind "route:#{ route_id }", ()=>
        @trigger.apply @, ["change:navigation", route_id  ].concat( _( arguments ).flatten() )

  #### Router Functions

  # Intercept calls to Backbone.Router.navigate so that we can at least
  # build a path from the route, even if we don't trigger the route handler
  navigate: (route, triggerRoute=false)->
    Backbone.Router.prototype.navigate.apply @, arguments
    @buildPathFrom( Backbone.history.getFragment() )

  # given a url fragment, construct an argument chain similar to what would be
  # emitted from a normal route:#{ name } event that gets triggered
  # when a route is actually fired.  This is used to trap route changes that happen
  # through calls to @navigate()
  buildPathFrom: (matchedRoute)->
    _(@routes).each (route_id, route)=>
      regex = @_routeToRegExp(route)
      if regex.test(matchedRoute)
        args = @_extractParameters(regex, matchedRoute)
        @trigger.apply @, ["change:navigation", route_id].concat( args )

What I do now is customize my router so that it emits route events every time somebody makes a call to router.navigate ( regardless if the trigger route param is set to true. ) Which, IMO, is an improvement over the stock backbone behavior

I almost always have a single application object ( a Luca.Viewport ) which all other views can bind to, or access to get global state. The application listens to change:navigation events, and then updates its internal state machine, so all interested parties can respond appropriately.

This eliminates the needs for before / after filters in the router, and moves the concern to where I think it is more appropriate, which is in your views.

@ghost

This comment has been minimized.

Show comment
Hide comment
@ghost

ghost May 31, 2012

I understand the framework just fine, and I continue to find that this puritanical argument has very little going for it. Given the fact that the router is the only component of Backbone I haven't completely replaced at this point, baddabing...

route: (route, name, callback) -> 
    super
    routeHandler = Backbone.history.handlers[0]
    routeHandler.callback = _.wrap routeHandler.callback, (originalCallback, fragment) =>
      try
        @trigger("myapp:routing", route, name, @)
        originalCallback.apply(@, _.toArray(arguments).slice(1))
        @trigger("myapp:routed", route, name, @)
      catch e
        console.error("Routing aborted: #{e}")

Works with the declarative syntax and lets routes object to running if desired.

ghost commented May 31, 2012

I understand the framework just fine, and I continue to find that this puritanical argument has very little going for it. Given the fact that the router is the only component of Backbone I haven't completely replaced at this point, baddabing...

route: (route, name, callback) -> 
    super
    routeHandler = Backbone.history.handlers[0]
    routeHandler.callback = _.wrap routeHandler.callback, (originalCallback, fragment) =>
      try
        @trigger("myapp:routing", route, name, @)
        originalCallback.apply(@, _.toArray(arguments).slice(1))
        @trigger("myapp:routed", route, name, @)
      catch e
        console.error("Routing aborted: #{e}")

Works with the declarative syntax and lets routes object to running if desired.

@posabsolute

This comment has been minimized.

Show comment
Hide comment
@posabsolute

posabsolute May 31, 2012

Well I see the point, it would work either way fo me but I still think that events or filters, it would be a great addition to the router

posabsolute commented May 31, 2012

Well I see the point, it would work either way fo me but I still think that events or filters, it would be a great addition to the router

@jaseemabid

This comment has been minimized.

Show comment
Hide comment
@jaseemabid

jaseemabid Jan 9, 2014

I'd vote for the feature. I came across this problem and opened another ticket #2947 (sorry for the duplicate). Simple use cases might be auth, making sure a parent view is rendered before a child view rendered by a deep router etc. Also, looks like the effort to add this feature is not much.

jaseemabid commented Jan 9, 2014

I'd vote for the feature. I came across this problem and opened another ticket #2947 (sorry for the duplicate). Simple use cases might be auth, making sure a parent view is rendered before a child view rendered by a deep router etc. Also, looks like the effort to add this feature is not much.

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