Ways to avoid forgetting to call this.refresh()

[jonrsawyer]

As I experiment with my Material Design components using Crank, I have repeatedly misled myself into thinking I had some sort of component definition problem, when in fact all I'd done is forgotten to call this.refresh(). Oops.

Any ideas on idioms to use to avoid this potentially common pitfall? Other than "be smarter"? :-)

In some ways it seems kinda like a cross-cutting concern that AOP may lend itself to, but that may be overkill.

I was thinking about the possibility of all "onevent" calls from the UI components could implicitly call this.refresh() after calling application code, but that may be a mistake for a variety of reasons: some UI events may not need to trigger a refresh, while other non-UI events will need to, and it may be even less obvious to application programmers why it is needed in some cases rather than others.

Conceptually, I'd like to "draw a bubble" around all application callbacks, and have a conceptual equivalent of try ... finally cause this.refresh() to be called (once!) upon completion of the callback(s). Ideally, a callback could "opt out" of the need for a refresh as an optimization somehow, but a default behavior of refreshing (instead of the current default of not refreshing when you don't call this.refresh()) seems to be a "safer" behavioral pattern. A concrete mechanism for this does not come to mind, however.

[brainkim]

I’m currently writing a blogpost talking about why I think an explicit refresh is better. In the meantime, you might want to read the thoughts various people have here: #37

I’m curious about the concrete workflow you had when you made the bug, what you were doing, how hard it was to discover the cause of the bug, and whether you have made the bug fewer times since getting used to the cranky way.

My theory is that explicit calls to this.refresh are good. They’re good for debuggability, and when they’re missing it’s immediately obvious, and it’s actually valuable feedback in the course of development because it teaches you “hey” this component isn’t rerendered by the parent and requires a refresh. But you can always consider explicit refreshes a low-level interface and build stateful things on top of the Crank context which calls refresh automatically, or you can use async generator components which continuously have values pulled. I’m collecting my thoughts about this but will write more here and in #37 when I can.

[jonrsawyer]

A concrete example: in my demo harness for the Material Design components, I have a menu structure like this:

                        <MenuAnchor>
                            <Button onclick={openMenu}>Menu</Button>
                            <Menu open={menuOpen}>
                                <MenuItem onclick={menuItemClicked}>Menu Item</MenuItem>
                                ...
                            </Menu>
                        </MenuAnchor>
                        <p>Menu item clicked {menuClickCount} times.</p>

And I have the following code:

        let menuClickCount = 0;
        const menuItemClicked = () => {
            menuClickCount++;
            this.refresh();                   // <== This was missing at first
        }

Note that this was while I was hooking up the behavior of the Menu* components themselves. Before I had the this.refresh() call in there, clicking the menu item did not increment the counter displayed in the <p>...</p> tag. At first, I thought I hadn't hooked up the callback properly.

This maybe wasn't a good example, though it was succinct. There were other times the missing this.refresh() was masked because in one sequence of events, one of the event handlers triggered a refresh, which in another sequence of events, only the event handler(s) that did not trigger a refresh was called.

Actually, another version of this problem looks like the following:

        <MenuItem onclick={() => menuClickCount++}>Menu Item</MenuItem>

In this idiom, for some reason, the need for an explicit this.refresh() is less obvious to my eyes. But maybe that's just me.

Now, I admit that I'm looking at this from the perspective of a component developer, where the components I'm working on are immediately suspect. The perspective of the application developer using a hopefully mostly vetted component library would probably focus more on their code, including these callbacks, thus the problem may be more evident.

I probably need to work with this longer, and get "crankier", as you put it, before this.refresh() will be second nature.

[jonrsawyer]

OK, just to stimulate the conversation (and probably annoy Brian... ;-)): here's an example of using a Proxy to wrap a UI data object to automate this.refresh().

    const menuData = {
        menuClickCount: 0
    }

    const menuDataProxy = new Proxy([menuData], {
        get: function (proxyTarget, propertyKey) {
            const foundParent = proxyTarget.find(parent => parent[propertyKey] !== undefined);
            return foundParent && foundParent[propertyKey];
        },

        set: function (proxyTarget, propertyKey, value) {
            const foundParent = proxyTarget.find(parent => parent[propertyKey] !== undefined);
            if (foundParent && foundParent[propertyKey] !== value) {
                foundParent[propertyKey] = value;
                thisApp.refresh();
            }
            return true; // Proxy contract
        }
    });

...

       <MenuItem onclick={() => menuDataProxy.menuClickCount++}>Menu Item</MenuItem>
        ...
       <p>Menu item clicked {menuDataProxy.menuClickCount} times.</p>

This works. A couple things to note:

• only the one setter and getter are needed on the proxy regardless of how many data values are stored in menuData;
• refresh() is only called if the value actually changes;
• a slightly more complex implementation could coalesce refresh() calls caused by setting multiple properties;
• repeated application of the proxy could be used to support hierarchical data, e.g. menuData.menu1.value, though this may involve slightly more complexity to perform that repeated application automatically.

As long as the UI components only reference data values via the proxy, which could be enforced using conventional Javascript scoping rules, refresh() will be called "automatically".

All of that said, I'm not yet convinced whether this is a generally useful idiom or not; Brian has been "cranky" a lot longer than I have and has thought about this a lot more, so I'm not ready to dismiss his admonition that explicitly calling refresh() is a good thing. However, having an alternative like this in my back pocket makes me feel better about proceeding with Crank, in case I eventually agree to disagree.

[brainkim]

You can probably abstract that proxy pattern to be like:

function createState(ctx: Context, initial: object = {}) {
  return new Proxy(initial, {
    /* whatever proxy logic you need to get stuff to refresh whenever you set a property I dunno I don’t use proxies and avoid them as much as I can haha */
  });
}

function *MyComponent() {
  const state = createState(this, {someState: "I dunno"});
  this.addEventListener("someevent", () => {
    state.someState = "Something else";
  });
  while (true) {
    yield <MyChild value={state.someState} />;
  }
}

This is not a personal endorsement of this kind of pattern, and I still think proxies are kinda icky, but it might be a useful sketch of the kinds of helper functions you can write, because I love experimentation!

However, having an alternative like this in my back pocket makes me feel better about proceeding with Crank, in case I eventually agree to disagree.

This is the sort of compromise I can get behind! Here’s some food for thought: If Crank also implemented its own internal state mgmt system, it would be slightly more difficult for you to extend Crank with your own state stuff, insofar as the internal rerendering logic might step on your state solution’s rerendering logic.

[jonrsawyer]

Proxies are just another tool in our tool chest. The comments you make about Proxies are probably made by other people about Generators. :-)

[jonrsawyer]

Second-guessing myself: is there even a need to coalesce this.refresh() calls? In other words, does this.refresh() set a flag so that some time later a refresh is done, or does it imperatively invoke the refresh process? If the former, multiple calls to this.refresh() are coalesced for me.

[brainkim]

Crank doesn’t do any coalescing of refresh calls unless the component is asynchronous. In the case of sync generators, it does silently fail and stop refreshing if you try to refresh while the generator is currently running. See #70 for more info. This is what would prevent errors when you accidentally triggered a refresh with your proxy from within the main loop of a sync generator.

[ryhinchey]

This is what’s exciting to me about crank. It’s inevitable that some type of library or pattern will emerge that refreshes your components automatically. We don’t know what that will look like yet but it’s clear that we can get there with core js language features and not a framework specific paradigm. I love the idea that as you write crank applications, you’ll probably educate yourself on core js concepts which are applicable outside of crank.

[mcjazzyfunky]

@jonrsawyer Not really sure whether this could be interesting for you ... anyway, here's a little demo using a library called "observable-slim" (version 0.1.5)
https://codesandbox.io/s/red-grass-nyx9f

[jonrsawyer]

Yes, that's very interesting! That's almost what I had envisioned for the Proxy behavior; the one exception being that a get() of a sub-object would wrap that object in a Proxy too, so the handling of sub-objects and their properties would be transparent w.r.t. render(). I'm going to check out observable-slim more...

[mcjazzyfunky]

@brainkim Frankly (sorry for being annoying again), sooner or later you really have to face (and answer) the question whether writing "Vue3/Hooks"-like helper functions in userland and using them over and over again with (a future version of) Crank is a proper way to implement Crank components or whether this is not cranky at all and should never be done.

// Important: All functions used in this example are written completely in userland,
// they are NOT part of the Crank core.
// In the current Crank version, it's not possible to implement most of these functions,
// but who knows, this may change in future. 

// The following is obviously highly inspired by Vue3 and React hooks
// (but completely non-magical).

function* Counter() {
  const
    props = propsWithDefaults(this, { label: 'Counter', initialCount: 0 }),
    state = reactive(this, { count: props.initialCount )),
    onIncrement = () => { ++state.count }
    
  onMounted(this, () => {
    console.log('Component has been mounted')
  })  
  
  onBeforeUnmount(this, () => {
    console.log('Component will be unmounted')
  })
  
  onUpdated(this, () => {
    console.log(`Value of "${props.label}": ${state.count}`)
  }, () => [state.count]) // this is similar to useEffect in React
  
  while (true) {
    yield <button onclick={onIncrement}>{props.label} {state.count}</button>
  }
})

[brainkim]

Good abstractions take time and foresight, but mostly, it’s about pattern recognition, which is impossible without reading and writing actual code. Before jumping to hooks, try writing out your logic the long way, see what code is produced, and meditate on how you might abstract it. I really really really dislike callbacks, because while they look like regular code blocks, their actual cyclomatic complexity is much much worse. Don’t worry, I’ve been thinking about this problem and have several solutions which won’t look like hooks, some of which are already available to us as users.

Also, I like how everyone is prefacing their feature suggestions with “I hope I’m not being annoying.” Don’t worry about being annoying. I appreciate the feedback.

[mcjazzyfunky]

@brainkim You have no idea, how much I am looking forward to hear more about your ideas and solutions regarding Crank's answer to hooks (quote: "which won't look like hooks") 👀 😉 ... this will be one of the most interesting and important parts of all Crank patterns (and will surely be also very interesting outside of the Crank world).
Please keep in mind that (at least IMHO) the most tricky part is providing coequal support for automatic type inference in TypeScript and also providing coequal power regarding composability like in the following snippet:

function SomeComponent___or___useSomething(...) {
  ...
  const
    x = useA(a),
    y = useB(1, x * 2,  b),
    z = useC(42, y, x)
  ...
}

But of course, I do not want to push (okay, maybe just a tiny little teeny-weeny bit 😉) ... a good wine needs time to settle and mature.