WebReflection/introspected

0.2.0 Missing initial update

albertosantini opened this issue · 9 comments

The example below displays an initial blank page and after a second the message "data updated".
It seems the initial update, rendering the message "data NOT updated", is not called.

"use strict";

// foo.template.js
class FooTemplate {
    static update(render, state) {
        render`<p> ${state.data} </p>`;
    }
}

// foo.controller.js
class FooController {
    constructor(render, template) {
        this.state = Introspected({
            data: "data NOT updated"
        }, state => template.update(render, state));

        setTimeout(() => {
            this.state.data = "data updated";
        }, 1000);
    }
}

// foo.component.js
class FooComponent {
    constructor() {
        const el = document.querySelector("foo");
        const render = hyperHTML.bind(el);

        this.fooController = new FooController(render, FooTemplate);
    }
}

// foo.module.js
new FooComponent();

Live example: https://plnkr.co/edit/JMQnl9qssBwxdj1ad3dp?p=preview

interesting, you are expecting an update call even if nothing was updated, correct?

It might make sense to notify with an empty path once after the initial import though, I'll update this.

Well, I noticed the diff w.r.t the previous version.

Same behaviour with

        this.state = Introspected({},
            state => template.update(render, state));

        this.state.data = "data NOT updated";

        setTimeout(() => {
            this.state.data = "data updated";
        }, 1000);

No, I was wrong: it is ok.

I see Introspected.observe = Introspected;.

Maybe it is an opinionated behaviour, but I think it is expected the initial update when the Introspected object is created: Introspect observes, froma semantic point of view, an empty state and then an initial one. :)

It might make sense to notify with an empty path once after the initial import though, I'll update this.

Agreed.

on a second thought, when you are introspecting some data you are in a setup phase, invoking the callback before the variable is even returned might cause undesired issues, delaying the first invocation might cause flicks.

Since by contract the callback is invoked when there is an update or change, I actually think you should do things differently:

  • you know the data is fully imported, render after
  • you import less data and you add or change things after, updates are triggered

Accordingly, I'd write your example as such:

const update = state => template.update(render, state);
this.state = Introspected(
  {data: "data NOT updated"},
  update
);
update();

This gives you full control on when you want to trigger the first update based on the imported state.

It seems a cleaner approach. Thoughts?

I see Introspected.observe = Introspected;.

It's mostly a migration pattern from 0.1 'cause now any introspected is observed by default but more callbacks can be added later on at any time.

Signature is now Introspected(serializable[, callback])

Should update the README too

look, we're playing around at this stage so I'll put in the version that triggers first callback via an empty path.

Maybe it's the most handy pattern after all, and it's easy to avoid such call simply not passing a callback and observing it later on explicitly (that invokes anyway on import).

it's in 0.2.1

Maybe it's the most handy pattern after all, and it's easy to avoid such call simply not passing a callback and observing it later on explicitly (that invokes anyway on import).

Agreed. Tested. Thanks.