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

multiple instance of Todos #25

Closed
yapcheahshen opened this issue Jun 16, 2012 · 18 comments
Closed

multiple instance of Todos #25

yapcheahshen opened this issue Jun 16, 2012 · 18 comments
Labels
Milestone

Comments

@yapcheahshen
Copy link

when the Todos is not empty, clicking "Start Todos" causes same data to be rendered .
version: Backbone Aura 0.8 Developer Preview
steps to repeat the problem:

  1. launch backbone-aura demo http://localhost/backbone-aura/aura/www/index.html
  2. enter something in TODO list
  3. click "Start Todos" button
@dustinboston
Copy link
Contributor

Good catch. Maybe we need a way to check the status if a widget in the
mediator (started, stopped)
On Jun 16, 2012 8:12 AM, "yap cheah shen" <
reply@reply.github.com>
wrote:

when the Todos is not empty, clicking "Start Todos" causes same data to be
rendered .
version: Backbone Aura 0.8 Developer Preview
steps to repeat the problem:

  1. launch backbone-aura demo
    http://localhost/backbone-aura/aura/www/index.html
  2. enter something in TODO list
  3. click "Start Todos" button

Reply to this email directly or view it on GitHub:
https://github.com/addyosmani/backbone-aura/issues/25

@addyosmani
Copy link
Member

I think exposing the status makes sense. started, stopped - anything else?

@addyosmani
Copy link
Member

@dustinboston I believe a similar issue was mentioned on another ticket. Should we be explicitly storing widget state? If so, where should this be stored? data? internal cache?

@dustinboston
Copy link
Contributor

@yapcheahshen - Can you clarify? I know this was a while ago, are you still experiencing the issue?
@addyosmani - If this was duplicate date being rendered, I believe we fixed this. It was quite some time ago. I think started/stopped is sufficient for now.

@yapcheahshen
Copy link
Author

@dustinboston
Copy link
Contributor

@yapcheahshen Video! Awesome! We'll expose the status and wrap the calls with a check for a loaded widget. Do you want to take a shot at it and send a Pull Request?

@addyosmani
Copy link
Member

@yapcheahshen ping, in case you're interested. If not, we'll try to assign to another member of the team that might be interested.

@yapcheahshen
Copy link
Author

@addyosmani i am not quite understood, how could i contribute ?
i use backbone.js everyday but very ignorant of how Github works.
most of my projects are on fossil-scm.

regards,
yap

2012/8/18 Addy Osmani notifications@github.com

@yapcheahshen https://github.com/yapcheahshen ping, in case you're
interested. If not, we'll try to assign to another member of the team that
might be interested.


Reply to this email directly or view it on GitHubhttps://github.com/addyosmani/backbone-aura/issues/25#issuecomment-7840777.

@addyosmani
Copy link
Member

@yapcheahshen Oh! So, the way Github works is quite straight-forward. On the main page of this project you'll see a button to the right called 'fork'. Clicking it will create a version of the project on your GitHub account that you can then edit. You would create a local clone of this (using git clone) and start making changes. When you're happy with your changes you can commit and push them to your remote copy. If you then go to your fork on GitHub, you'll see a button called 'pull request' where you can submit a pull request to this project.

See https://help.github.com/articles/fork-a-repo and https://help.github.com/articles/using-pull-requests/

@yapcheahshen
Copy link
Author

thanks for your instruction, i fork and clone a local copy of backbone-aura.

i can prevent multiple instance of todo item by adding one line in
src/widgets/todos/view/app.js

initialize: function () {
if (this.$el.children('').length) return; // do not initialize the view

but i think this is only a quick hack and not generally a good practice,
haven't dig into detail implementation of aura yet.
I wonder if you keep something like a "reference count" for each backbone
view in mediator ?
if so, we might need a flag in the mediator.start to allow/disallow user to
create some view when the view is already exists.

regards,
yap

2012/8/18 Addy Osmani notifications@github.com

@yapcheahshen https://github.com/yapcheahshen Oh! So, the way Github
works is quite straight-forward. On the main page of this project you'll
see a button to the right called 'fork'. Clicking it will create a version
of the project on your GitHub account that you can then edit. You would
create a local clone of this (using git clone) and start making changes.
When you're happy with your changes you can commit and push them to your
remote copy. If you then go to your fork on GitHub, you'll see a button
called 'pull request' where you can submit a pull request to this project.

See https://help.github.com/articles/fork-a-repo and
https://help.github.com/articles/using-pull-requests/


Reply to this email directly or view it on GitHubhttps://github.com/addyosmani/backbone-aura/issues/25#issuecomment-7843761.

@addyosmani
Copy link
Member

@yapcheahshen if you're still interested in putting together a pull request, we'd definitely be interested in you exploring this further :)

@addyosmani
Copy link
Member

I wonder if you keep something like a "reference count" for each backbone view in mediator

We don't at present, but could certainly do something like that. Would it make sense to just store references with a type of hash/ID/sha or something similar rather than count? We could then check to see if a single instance existed and handle appropriately. would that work?

@yapcheahshen
Copy link
Author

yes, object references is better,
reference count is static language convention..

regards,
yap

2012/8/25 Addy Osmani notifications@github.com

I wonder if you keep something like a "reference count" for each backbone
view in mediator

We don't at present, but could certainly do something like that. Would it
make sense to just store references with a type of hash/ID/sha or something
similar rather than count? We could then check to see if a single instance
existed and handle appropriately. would that work?


Reply to this email directly or view it on GitHubhttps://github.com/addyosmani/aura/issues/25#issuecomment-8014646.

@gersongoulart
Copy link
Contributor

Hey Guys!!!

I was revisiting this problem just now. And I think it is bigger than it seems.

On /src/widgets/todos/views/app.js line 25 the "initialize" function of the todos "AppView" binds a few events to the Todos Collection and to the sandbox every time this view is initialized (on start). That causes three problems:

The first is the one already described (even with video :D ) by @yapcheahshen, creating an extra entry of a todo for as many times as you click the "Start Todos" button. So let's say we click it three times, you'll get three entries of each todo.

The second is that the same thing happens when you add a new todo. (Say that you click the "Start Todos" button three times. Than you create a new todo, this new todo will be inserted in the list three times).

The third is not noticeable, but when you reset the todos, it also gets reset three times.

The reason why it happens is because the events "add", "reset" and "all" were bound three times to the Todo collection, the same way the listeners "new-event" and "todos" were added three times to the sandbox (not sure te impact it causes though).

Now that we have a little better understanding of the problem we could say that this is not exactly a bug in the todo app, but that something is lacking in Aura's architecture. We have a situation where widgets may re-bind events that were already bound. Now, why is this situation happening? Should aura have unbound events when the widget was stopped? Should it have completely got rid of all widget objects (Collections, Models, etc of the widget on stop)? Or should it simply prevent widgets of binding the same event with the same channel and subscriber more than once?

I'm really not sure which way to go. But I believe we must solve this issue before we can call Aura ready for prime time (v1).

@atesgoral
Copy link
Member

As far as "opinionated" goes, Aura could simply prevent the same widget from subscribing to the same channel more than once (your very last suggestion). I don't know if that could lead to laziness on the developers part, and lead to other problems down the line though...

Or perhaps widget-to-widget subscription could be made declarative, so it's entirely managed by Core...

@addyosmani
Copy link
Member

As far as "opinionated" goes, Aura could simply prevent the same widget from subscribing to the same channel more than once (your very last suggestion).

I would be tempted for us to opt for this option in the short term and explore whether there's a more solid solution that can avoid problems with laziness/confusion long-term.

@gersongoulart
Copy link
Contributor

@atesgoral, @addyosmani, thinking about this it seems to me that @dustinboston 's first suggestion that

we need a way to check the status if a widget in the mediator (started, stopped)

would be a cleaner solution for this problem. This way we would never start a widget instance more than once. Having one sandbox per instance of a widget, the sandbox could do this check if the widget is already start and prevent restart. That solves one problem.

But the fact that all that baggage of Models and Collections are still around (in the memory) after a widget has been stopped is bothering me big time. Let's say that we did not have the start/stop controls and that the todo widget was being stopped by the application because really it's not necessary anymore. All that code would stick around taking up memory? What if I had this seriously large web application that starts and stops many widgets every time the user browse different areas (think the yahoo portal as a whole in one page that never refreshes)? How much memory and processing power would be necessary for this app not to crash due to all objects left behind by the modules?

@addyosmani
Copy link
Member

We will no longer be using the Todo application as a demo as we've landed a new version of Aura in master which will (eventually) have a different app for the example.

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

No branches or pull requests

5 participants