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

Data repeat for ever revisit of the component #14

Closed
steff1193 opened this issue Oct 13, 2016 · 7 comments
Closed

Data repeat for ever revisit of the component #14

steff1193 opened this issue Oct 13, 2016 · 7 comments

Comments

@steff1193
Copy link

steff1193 commented Oct 13, 2016

In the README you tell how to use the component i Angular2

const Tasks = new MongoObservable.Collection<Task>('tasks');

@Component({
  selector: 'task-list',
  template: `<ul><li *ngFor="let task of tasks | async"></li></ul>`
})
class Tasks {
  tasks = Tasks.find().zone();
}

It seems to me that data will be repeated an extra time for every revisit to this component. That is

  • The first time you visit the component the data from "tasks" is shown once
  • The second time you visit the component the data from "tasks" is shown twice
  • ...
  • The n'th time you visit the component the data from "tasks" is shown n times

bsliran/angular2-meteor-base uses this code-pattern in the DemoComponent. In repeated-data (note branch "repeated-data") I have forked and added an extra component DemoComponent2, and set it up so that you can navigate back and forth between showing DemoComponent and DemoComponent2 (see this commit). After starting the application and pointing your browser to http://localhost:3000, if you click the "Demo"-link you will see data

  • Dotan (25)
  • Liran (26)
  • Uri (30)

If then click the "Demo 2"-link, and then the "Demo"-link you will see data

  • Dotan (25)
  • Liran (26)
  • Uri (30)
  • Dotan (25)
  • Liran (26)
  • Uri (30)

Taking another round and you will see data

  • Dotan (25)
  • Liran (26)
  • Uri (30)
  • Dotan (25)
  • Liran (26)
  • Uri (30)
  • Dotan (25)
  • Liran (26)
  • Uri (30)

I believe it has something to do with meteor-rxjs, but I am not sure. At least the problem does not occur if you are using a hardcoded data-array and no async (see this commit)

Hopefully you can help find out what is going wrong. If it is not a bug, then at least it would be nice if the README of meteor-rxjs under "Usage in Angular 2" mentioned something about this issue, and how to get around it.

@barbatus
Copy link
Collaborator

What if you add this.data.unsubscribe() on ngOnDestroy()?

@steff1193
Copy link
Author

Adding

  ngOnDestroy() {
    this.data.unsubscribe();
  }

to DemoComponent does not help, because unsubscribe is not a function on Observable.

EXCEPTION: Uncaught (in promise): TypeError: this.data.unsubscribe is not a functionmodules.js:44596:13
require<.node_modules["@angular"].core.bundles["core.umd.js"]</</ErrorHandler</ErrorHandler.prototype.handleError()modules.js:44596
require<.node_modules["@angular"].core.bundles["core.umd.js"]</</PlatformRef_</PlatformRef_.prototype._bootstrapModuleFactoryWithZone/</<.next()modules.js:48070
require<.node_modules["@angular"].core.bundles["core.umd.js"]</</EventEmitter</EventEmitter.prototype.subscribe/schedulerFn<()modules.js:47319
require<.node_modules.rxjs["Subscriber.js"]</SafeSubscriber</SafeSubscriber.prototype.__tryOrUnsub()modules.js:2801
require<.node_modules.rxjs["Subscriber.js"]</SafeSubscriber</SafeSubscriber.prototype.next()modules.js:2750
require<.node_modules.rxjs["Subscriber.js"]</Subscriber</Subscriber.prototype._next()modules.js:2703
require<.node_modules.rxjs["Subscriber.js"]</Subscriber</Subscriber.prototype.next()modules.js:2667
require<.node_modules.rxjs["Subject.js"]</Subject</Subject.prototype.next()modules.js:1393
require<.node_modules["@angular"].core.bundles["core.umd.js"]</</EventEmitter</EventEmitter.prototype.emit()modules.js:47311
NgZone/this._zoneImpl<.onError()modules.js:47535
NgZoneImpl/this.inner<.onHandleError()modules.js:47410
require<.node_modules["zone.js"].dist["zone.js"]/Zone$1</ZoneDelegate</ZoneDelegate.prototype.handleError()modules.js:21987
require<.node_modules["zone.js"].dist["zone.js"]/Zone$1</Zone</Zone.prototype.runGuarded()modules.js:21893
drainMicroTaskQueue/_loop_1()modules.js:22159
drainMicroTaskQueue()modules.js:22166
ZoneTask/this.invoke()modules.js:22088

ORIGINAL STACKTRACE:modules.js:44601:17
require<.node_modules["@angular"].core.bundles["core.umd.js"]</</ErrorHandler</ErrorHandler.prototype.handleError()modules.js:44601
require<.node_modules["@angular"].core.bundles["core.umd.js"]</</PlatformRef_</PlatformRef_.prototype._bootstrapModuleFactoryWithZone/</<.next()modules.js:48070
require<.node_modules["@angular"].core.bundles["core.umd.js"]</</EventEmitter</EventEmitter.prototype.subscribe/schedulerFn<()modules.js:47319
require<.node_modules.rxjs["Subscriber.js"]</SafeSubscriber</SafeSubscriber.prototype.__tryOrUnsub()modules.js:2801
require<.node_modules.rxjs["Subscriber.js"]</SafeSubscriber</SafeSubscriber.prototype.next()modules.js:2750
require<.node_modules.rxjs["Subscriber.js"]</Subscriber</Subscriber.prototype._next()modules.js:2703
require<.node_modules.rxjs["Subscriber.js"]</Subscriber</Subscriber.prototype.next()modules.js:2667
require<.node_modules.rxjs["Subject.js"]</Subject</Subject.prototype.next()modules.js:1393
require<.node_modules["@angular"].core.bundles["core.umd.js"]</</EventEmitter</EventEmitter.prototype.emit()modules.js:47311
NgZone/this._zoneImpl<.onError()modules.js:47535
NgZoneImpl/this.inner<.onHandleError()modules.js:47410
require<.node_modules["zone.js"].dist["zone.js"]/Zone$1</ZoneDelegate</ZoneDelegate.prototype.handleError()modules.js:21987
require<.node_modules["zone.js"].dist["zone.js"]/Zone$1</Zone</Zone.prototype.runGuarded()modules.js:21893
drainMicroTaskQueue/_loop_1()modules.js:22159
drainMicroTaskQueue()modules.js:22166
ZoneTask/this.invoke()modules.js:22088

resolvePromise@http://localhost:3000/packages/modules.js?hash=18f4829dbd2d3b6d0e24eee8e3b2fe2c027e4e1d:22209:31
makeResolver/<@http://localhost:3000/packages/modules.js?hash=18f4829dbd2d3b6d0e24eee8e3b2fe2c027e4e1d:22186:13
require<.node_modules["zone.js"].dist["zone.js"]/Zone$1</ZoneDelegate</ZoneDelegate.prototype.invoke@http://localhost:3000/packages/modules.js?hash=18f4829dbd2d3b6d0e24eee8e3b2fe2c027e4e1d:21983:19
NgZoneImpl/this.inner<.onInvoke@http://localhost:3000/packages/modules.js?hash=18f4829dbd2d3b6d0e24eee8e3b2fe2c027e4e1d:47389:32
require<.node_modules["zone.js"].dist["zone.js"]/Zone$1</ZoneDelegate</ZoneDelegate.prototype.invoke@http://localhost:3000/packages/modules.js?hash=18f4829dbd2d3b6d0e24eee8e3b2fe2c027e4e1d:21982:19
require<.node_modules["zone.js"].dist["zone.js"]/Zone$1</Zone</Zone.prototype.run@http://localhost:3000/packages/modules.js?hash=18f4829dbd2d3b6d0e24eee8e3b2fe2c027e4e1d:21876:24
scheduleResolveOrReject/<@http://localhost:3000/packages/modules.js?hash=18f4829dbd2d3b6d0e24eee8e3b2fe2c027e4e1d:22242:52
require<.node_modules["zone.js"].dist["zone.js"]/Zone$1</ZoneDelegate</ZoneDelegate.prototype.invokeTask@http://localhost:3000/packages/modules.js?hash=18f4829dbd2d3b6d0e24eee8e3b2fe2c027e4e1d:22016:23
NgZoneImpl/this.inner<.onInvokeTask@http://localhost:3000/packages/modules.js?hash=18f4829dbd2d3b6d0e24eee8e3b2fe2c027e4e1d:47380:32
require<.node_modules["zone.js"].dist["zone.js"]/Zone$1</ZoneDelegate</ZoneDelegate.prototype.invokeTask@http://localhost:3000/packages/modules.js?hash=18f4829dbd2d3b6d0e24eee8e3b2fe2c027e4e1d:22015:23
require<.node_modules["zone.js"].dist["zone.js"]/Zone$1</Zone</Zone.prototype.runTask@http://localhost:3000/packages/modules.js?hash=18f4829dbd2d3b6d0e24eee8e3b2fe2c027e4e1d:21916:28
drainMicroTaskQueue@http://localhost:3000/packages/modules.js?hash=18f4829dbd2d3b6d0e24eee8e3b2fe2c027e4e1d:22148:25
ZoneTask/this.invoke@http://localhost:3000/packages/modules.js?hash=18f4829dbd2d3b6d0e24eee8e3b2fe2c027e4e1d:22088:25
modules.js:44602:17

Unhandled Promise rejection: this.data.unsubscribe is not a function ; Zone: angular ; Task: Promise.then ; Value: TypeError: this.data.unsubscribe is not a function
Stack trace:
[object Object] require<.client.imports.app.demo["demo.component.js"]</DemoComponent</DemoComponent.prototype.ngOnDestroy@http://localhost:3000/app/app.js?hash=432b1fc730449d70a2e97f77a3e2d8a936ba2951:75:9
anonymous/_View_DemoComponent_Host0.prototype.destroyInternal@DemoComponent_Host.ngfactory.js:32:3
require<.node_modules["@angular"].core.bundles["core.umd.js"]</</AppView</AppView.prototype.destroyLocal@http://localhost:3000/packages/modules.js?hash=18f4829dbd2d3b6d0e24eee8e3b2fe2c027e4e1d:50647:13
require<.node_modules["@angular"].core.bundles["core.umd.js"]</</AppView</AppView.prototype._destroyRecurse@http://localhost:3000/packages/modules.js?hash=18f4829dbd2d3b6d0e24eee8e3b2fe2c027e4e1d:50635:13
require<.node_modules["@angular"].core.bundles["core.umd.js"]</</AppView</AppView.prototype.destroy@http://localhost:3000/packages/modules.js?hash=18f4829dbd2d3b6d0e24eee8e3b2fe2c027e4e1d:50621:13
require<.node_modules["@angular"].core.bundles["core.umd.js"]</</ComponentRef_</ComponentRef_.prototype.des[…]modules.js:22135:13

Error: Uncaught (in promise): TypeError: this.data.unsubscribe is not a function
Stack trace:
resolvePromise@http://localhost:3000/packages/modules.js?hash=18f4829dbd2d3b6d0e24eee8e3b2fe2c027e4e1d:22209:31
makeResolver/<@http://localhost:3000/packages/modules.js?hash=18f4829dbd2d3b6d0e24eee8e3b2fe2c027e4e1d:22186:13
require<.node_modules["zone.js"].dist["zone.js"]/Zone$1</ZoneDelegate</ZoneDelegate.prototype.invoke@http://localhost:3000/packages/modules.js?hash=18f4829dbd2d3b6d0e24eee8e3b2fe2c027e4e1d:21983:19
NgZoneImpl/this.inner<.onInvoke@http://localhost:3000/packages/modules.js?hash=18f4829dbd2d3b6d0e24eee8e3b2fe2c027e4e1d:47389:32
require<.node_modules["zone.js"].dist["zone.js"]/Zone$1</ZoneDelegate</ZoneDelegate.prototype.invoke@http://localhost:3000/packages/modules.js?hash=18f4829dbd2d3b6d0e24eee8e3b2fe2c027e4e1d:21982:19
require<.node_modules["zone.js"].dist["zone.js"]/Zone$1</Zone</Zone.prototype.run@http://localhost:3000/packages/modules.js?hash=18f4829dbd2d3b6d0e24eee8e3b2fe2c027e4e1d:21876:24
scheduleResolveOrReject/<@http://localhost:3000/packages/modules.js?hash=18f4829dbd2d3b6d0e24eee8e3b2fe2c027e4e1d:22242:52
require<.node_modules["zone.js"].dist["zone.js"]/Zone$1</ZoneDelegate</ZoneDelegate.prototype.invokeTask@http://localhost:3000/packages/modules.js?hash=18f4829dbd2d3b6d0e24eee8e3b2fe2c027e4e1d:22016:23
NgZoneImpl/this.inner<.onInvokeTask@http://localhost:3000/packages/modules.js?hash=18f4829dbd2d3b6d0e24eee8e3b2fe2c027e4e1d:47380:32
require<.node_modules["zone.js"].dist["zone.js"]/Zone$1</ZoneDelegate</ZoneDelegate.prototype.invokeTask@http://localhost:3000/packages/modules.js?hash=18f4829dbd2d3b6d0e24eee8e3b2fe2c027e4e1d:22015:23
require<.node_modules["zone.js"].dist["zone.js"]/Zone$1</Zone</Zone.prototype.runTask@http://localhost:3000/packages/modules.js?hash=18f4829dbd2d3b6d0e24eee8e3b2fe2c027e4e1d:21916:28
drainMicroTaskQueue@http://localhost:3000/packages/modules.js?hash=18f4829dbd2d3b6d0e24eee8e3b2fe2c027e4e1d:22148:25
ZoneTask/this.invoke@http://localhost:3000/packages/modules.js?hash=18f4829dbd2d3b6d0e24eee8e3b2fe2c027e4e1d:22088:25

@barbatus
Copy link
Collaborator

The reason is in that you re-use the same instance of the cursor observable (that comes from the service). I don't know whether it's good or bad, but I'd personally re-recreate new cursor each time with the component for clarity. Though it's fixed now for your case as well.

steff1193 added a commit to steff1193/angular2-ionic2-meteor-base that referenced this issue Oct 16, 2016
@steff1193
Copy link
Author

steff1193 commented Oct 16, 2016

I do not know if this approach is good or bad either, but it is kinda the approach introduced in the repo from which I forked: https://github.com/bsliran/angular2-meteor-base. To me it seem like a fair idea to reuse. I general it seems like at good idea to keep subscriptions open on things you know you want all the time, even though you are not currently showing it. E.g. a facebook-ish app, it seems like a good idea to keep the subscription open to the flow of posts that goes onto the main list of posts, even though you are not currently looking at it, e.g. if you clicked on one of the posts to look at that? I am new to meteor, angular etc, so I do not know about how to do it "the right way".

But more importantly, the fix you made does not seem to work. I updated my code to depend on the latest from this repo (see this commit on "repeated data" branch), but the behavior is still that data is repeated for each revisit of the Demo page.
I see that "the fix" is adding this._data = [] to ObservableCursor.stop. How is that supposed to fix the problem? Who is supposed to call ObservableCursor.stop. I know I am annoying, but I would really appreciate a fix, or maybe an "explanation for dummies" on how to work around the problem.

@barbatus
Copy link
Collaborator

Because 'prepublish' script is not run when installed from Github. Go to node_modules/meteor-rxjs and run npm run build-only.

On who stops the observable. It stopes itself when the last subscription unsubscribes from it.
Particularly it happens when the observable is passed to AsyncPipe, so it does let sub = this.data.subscribe(...) on ngOnInit and sub.unsubscribe() on ngOnDestroy.

@steff1193
Copy link
Author

Seems to solve the problem. Thanks a alot!

Any idea when you will create a release including this fix?
I was wondering why this repo does not have any tags. Dont you tag when you release?

@barbatus
Copy link
Collaborator

i'll probably add some minor stuff and then ask Uri to release (this week likely).
We use tags but this package is too simple yet for them.

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

2 participants