tensorflow / tensorboard-plugin-example

Apache License 2.0
135 stars 59 forks source link

greeter-card observers #5

Closed BinhangYuan closed 7 years ago

BinhangYuan commented 7 years ago

Hi,

I just get started with Polymer, so this question may seem naive.

I found that Line 81 in the greeter-card.html is confusing to me:

observers: ["reload(run, tag)"],

I guess here it tries to call the member function in Line 94:

reload() {
        if (!this._attached) {
          return;
        }
        this._canceller.cancelAll();
        const router = getRouter();
        const url = router.pluginRunTagRoute("greeter", "/greetings")(
          this.tag, this.run);
        const updateData = this._canceller.cancellable(result => {
          if (result.cancelled) {
            return;
          }
          const backendData = result.value;
          this.greetings = backendData;
        });
        this.requestManager.request(url).then(updateData);
      }

It seems like the parameters do not match here. There is no parameter for the function definition, while when observers tries to call this method, it passed run and tag, the public member properties of this Web component. I am wondering if this is problematic.

BTW, I tried to change Line 91 to:

observers: ["reload()"],

It still works for this simple demo.

BinhangYuan commented 7 years ago

Any suggestions? Thanks!

chihuahua commented 7 years ago

@BinhangYuan, sorry for missing this.

The list of observers specifies what parameters are being observed, in this case run and tag. Hence, every time either run or tag is set, the reload method is run.

You are correct in that the definition of reload accepts no arguments. The best practice would be to use reload(run, tag) (which I will fix in a pull request). However, the logic as written should still work.

Removing the parameters from instance in the observers list probably breaks behavior because the reload method won't be called when either the run or tag properties change.

Thank you for reporting this issue!