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

Make Backbone.history.historyStarted accessible to developers. #538

Closed
wants to merge 1 commit into
base: master
from

Conversation

Projects
None yet
10 participants
@krunkosaurus

krunkosaurus commented Aug 4, 2011

Just a minor edit.

In my example I call Backbone.history.start really late because of Facebook authentication reasons and when a user's session is disconnected I must call an .init script again where I include a check to see if we're already listening for hash changes.

@tbranyen

This comment has been minimized.

Show comment
Hide comment
@tbranyen

tbranyen Aug 17, 2011

Collaborator

This has been discussed in IRC and while I think this should be pulled, there are conflicting opinions. I'll mark this for needs review. I think opinions should be weighed in.

My personal opinion is that there should be some mechanism for finer control over Backbone history and perhaps a documented interface into the internals may be better in case historyStarted changes in the future.

Collaborator

tbranyen commented Aug 17, 2011

This has been discussed in IRC and while I think this should be pulled, there are conflicting opinions. I'll mark this for needs review. I think opinions should be weighed in.

My personal opinion is that there should be some mechanism for finer control over Backbone history and perhaps a documented interface into the internals may be better in case historyStarted changes in the future.

@shakerlxxv

This comment has been minimized.

Show comment
Hide comment
@shakerlxxv

shakerlxxv Aug 31, 2011

I would agree there should be some mechanism to determine if history has been started, perhaps an option to the start method would be more appropriate.

In my case different Backbone routers can be used on the page depending on which dialog box is opened. When these close one dialog box, what I really like to have is Backbone.history.stop() which would then allow the next dialog box the user might interact with to call Backbone.history.start() naively.

As it stands, I believe I'm left with the following option

try {
  Backbone.history.start()
} catch(err) {
  Backbone.history.loadUrl()
}

shakerlxxv commented Aug 31, 2011

I would agree there should be some mechanism to determine if history has been started, perhaps an option to the start method would be more appropriate.

In my case different Backbone routers can be used on the page depending on which dialog box is opened. When these close one dialog box, what I really like to have is Backbone.history.stop() which would then allow the next dialog box the user might interact with to call Backbone.history.start() naively.

As it stands, I believe I'm left with the following option

try {
  Backbone.history.start()
} catch(err) {
  Backbone.history.loadUrl()
}
@jasonm

This comment has been minimized.

Show comment
Hide comment
@jasonm

jasonm Sep 30, 2011

Contributor

This private state makes it a little ugly to isolation test backbone applications (e.g. with Jasmine), since I have to wrap .start() in a try/catch:

var ExampleApp = {
  Models: {},
  Collections: {},
  Views: {},
  Routers: {},
  init: function(tasks) {
    new ExampleApp.Routers.Tasks();
    this.tasks = new ExampleApp.Collections.Tasks(tasks);
    try {
      Backbone.history.start()
    } catch(exception) {}
  }
};

I would also be happy Backbone.history.stop(), which I could run after each relevant test case.

Contributor

jasonm commented Sep 30, 2011

This private state makes it a little ugly to isolation test backbone applications (e.g. with Jasmine), since I have to wrap .start() in a try/catch:

var ExampleApp = {
  Models: {},
  Collections: {},
  Views: {},
  Routers: {},
  init: function(tasks) {
    new ExampleApp.Routers.Tasks();
    this.tasks = new ExampleApp.Collections.Tasks(tasks);
    try {
      Backbone.history.start()
    } catch(exception) {}
  }
};

I would also be happy Backbone.history.stop(), which I could run after each relevant test case.

@timoxley

This comment has been minimized.

Show comment
Hide comment
@timoxley

timoxley Oct 10, 2011

+1. I too am having to wrap this in a try/catch for running in a test environment, which makes code uglies.

timoxley commented Oct 10, 2011

+1. I too am having to wrap this in a try/catch for running in a test environment, which makes code uglies.

@evil-shrike

This comment has been minimized.

Show comment
Hide comment
@evil-shrike

evil-shrike commented Nov 29, 2011

+1 - history.stop is needed for unit-testing
https://groups.google.com/forum/?pli=1#!topic/backbonejs/kZR9X2PXW7o

@eastridge

This comment has been minimized.

Show comment
Hide comment
@eastridge

eastridge Jan 22, 2012

Contributor

+1

Contributor

eastridge commented Jan 22, 2012

+1

@jashkenas jashkenas closed this in 96a7274 Jan 23, 2012

@jashkenas

This comment has been minimized.

Show comment
Hide comment
@jashkenas

jashkenas Jan 23, 2012

Owner

Thanks for working out the proper approach -- exposing historyStarted directly definitely isn't appropriate, but I've added Backbone.history.stop(), which you can use to disable history temporarily.

Owner

jashkenas commented Jan 23, 2012

Thanks for working out the proper approach -- exposing historyStarted directly definitely isn't appropriate, but I've added Backbone.history.stop(), which you can use to disable history temporarily.

@parliament718

This comment has been minimized.

Show comment
Hide comment
@parliament718

parliament718 Apr 6, 2013

A situation where historyStarted is needed and stop() will not do:

Using TypeScript to "simulate" inheritance of a BaseRouter class ("simulate" because after all it has a pure Javascript manifestation) that calls start() in its constructor. Now inheriting from this class more than once becomes a problem because the second derived class (or a second instance of the same derived class) will throw an error when it calls its base constructor (upon instantiation).

I have precisely this problem now and am forced to use this monstrosity in the base constructor:

  try{ Backbone.history.start({ pushState: true }); } catch(barf) { ; }

parliament718 commented Apr 6, 2013

A situation where historyStarted is needed and stop() will not do:

Using TypeScript to "simulate" inheritance of a BaseRouter class ("simulate" because after all it has a pure Javascript manifestation) that calls start() in its constructor. Now inheriting from this class more than once becomes a problem because the second derived class (or a second instance of the same derived class) will throw an error when it calls its base constructor (upon instantiation).

I have precisely this problem now and am forced to use this monstrosity in the base constructor:

  try{ Backbone.history.start({ pushState: true }); } catch(barf) { ; }
@akre54

This comment has been minimized.

Show comment
Hide comment
@akre54

akre54 Apr 6, 2013

Collaborator

@parliament718 Backbone.History.started is available for you to do your own checking.

Try this:

if (!Backbone.History.started) Backbone.history.start({ pushstate: true} );
Collaborator

akre54 commented Apr 6, 2013

@parliament718 Backbone.History.started is available for you to do your own checking.

Try this:

if (!Backbone.History.started) Backbone.history.start({ pushstate: true} );
@tbranyen

This comment has been minimized.

Show comment
Hide comment
@tbranyen

tbranyen Apr 6, 2013

Collaborator

@parliament718 That seems like a flaw in TypeScript, it shouldn't be automatically calling up super.

Collaborator

tbranyen commented Apr 6, 2013

@parliament718 That seems like a flaw in TypeScript, it shouldn't be automatically calling up super.

@parliament718

This comment has been minimized.

Show comment
Hide comment
@parliament718

parliament718 Apr 9, 2013

@akre54

I just noticed this has the very peculiar effect of adding a "#" to each of my routes.

As in after I change from my ugly try/catch blocks to check Backbone.History.started:

if (!(Backbone.History).started) Backbone.history.start({ pushstate: true} );
//try{ Backbone.history.start({ pushState: true }); } catch(e) { ; }

All of my routes end up as "site.com/#Controller" rather than "site.com/Controller".

I have toggled between the 2 lines several times and each time correctly confirmed it's exactly that switch that causes this.

I'm switching back to the try/catch for now until this can be resolved.

Thanks

parliament718 commented Apr 9, 2013

@akre54

I just noticed this has the very peculiar effect of adding a "#" to each of my routes.

As in after I change from my ugly try/catch blocks to check Backbone.History.started:

if (!(Backbone.History).started) Backbone.history.start({ pushstate: true} );
//try{ Backbone.history.start({ pushState: true }); } catch(e) { ; }

All of my routes end up as "site.com/#Controller" rather than "site.com/Controller".

I have toggled between the 2 lines several times and each time correctly confirmed it's exactly that switch that causes this.

I'm switching back to the try/catch for now until this can be resolved.

Thanks

@akre54

This comment has been minimized.

Show comment
Hide comment
@akre54

akre54 Apr 10, 2013

Collaborator

That is peculiar. Can you make a jsfiddle or similar showing just this bug
in action? Thanks!

Collaborator

akre54 commented Apr 10, 2013

That is peculiar. Can you make a jsfiddle or similar showing just this bug
in action? Thanks!

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