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

Computations creating signals #21

Closed
mrjjwright opened this issue Mar 10, 2019 · 10 comments
Closed

Computations creating signals #21

mrjjwright opened this issue Mar 10, 2019 · 10 comments

Comments

@mrjjwright
Copy link

mrjjwright commented Mar 10, 2019

If a computation computes new signals, i.e. new state e.g. by computing a new S.data signal, are there any rules of which to be aware? I haven't been successful in exactly understanding the rules. In a lot of cases it works, but in some rare cases I have to create a new S.root and dispose that root manually in the cleanup method of the computation.

Most of the high level state that drives the rest of the computation of an app can be created in the main S.root but how to handle cases where you need to create new signals?

@adamhaile
Copy link
Owner

Data signals -- what you get from either S.data() or S.value() -- can be created anywhere, and they don't have any special lifecycle. They're just like any js object,. You can create, pass, and use them anywhere. They'll get garbage collected once they're no longer referenced.

Can you say more about the "rare cases" where you saw weird behavior?

I might have a guess. If you're creating data signals inside computations, the part that sometimes gets tricky is that if the computation updates, new data signals are created, which therefore reset to whatever initial state is assigned, making it seem like their state never changes.

For instance, I once made a component that looked about like this:

function ClickMe() {
    const clicked = S.data(false);
    return clicked() ? <div>Clicked</div> : <div onClick={() => clicked(true)}>Click Me</div>;
}

So the intent there is that it creates a div that says "Click Me" until you click it, at which point it creates a div that says "Clicked."

But that's not what happens :). Since <ClickMe /> doesn't just create but also reads clicked(), it has a dependency on clicked(). So when we change clicked(), we think we're saying switch to the other div, but we're actually saying re-run <ClickMe />. That re-run creates a new clicked() signal, which starts out with the initial state we picked, false. So the component never switches to the "Clicked" state and always says "Click Me."

The real problem was that I mis-defined <ClickMe />. I wanted it to switch between two divs, which means its output should be a signal, not a static node. Signals can change, but the only way to change a static node is to re-run the entire function. So we can fix it by having it return a computation that picks between the two divs:

function ClickMe() {
    const clicked = S.data(false);
    return S(() => clicked() ? <div>Clicked</div> : <div onClick={() => clicked(true)}>Click Me</div>);
}

Does that sound relevant to your situation? If you have a code sample where data signals created in computations weren't working the way you thought they would, post it up and I'll take a look.

@ryansolid
Copy link

ryansolid commented Mar 10, 2019

EDIT: Follow Adam's comment first. It's more likely to help you with your specific issue. I go into explaining a bunch of stuff that is probably unnecessary to solve your problem.


It's not the nested signals but the nested computations that are of interest here. Signals can freely be collected when they move out of scope. However with computations it's possible to create dependencies that exist outside of their calling scope. If they are not disposed further changes to signals will continue to trigger them even after you've lost variable reference to them. Luckily S is setup so that on every computation re-run and disposal it disposes of all dependencies and child computations. As you can imagine this cascades since the disposal of the child computation disposes it's dependencies and it's child computations and so on.

So this holds up in the majority of cases. But it also means you are always redoing the work. So consider a conditional in view (this isn't exactly how it works but I think it conveys the point):

const count = S.data(6);
const parentNode = document.createElement('div')

S(() => {
  if (count() > 5) {
    parentNode.textContent = '';
    const node = document.createElement('div');
    S(() => node.textContent = 'Tracked ' + count() + ' times.');
    parentNode.appendChild(node);
  } else {
    parentNode.textContent = 'Too Low';
  }
});

Now your intention here is to only draw the child if the count is bigger than 5 and then update the count in the child div as the value changes. However every time the count changes the outer computation retriggers and every thing is redone. It is possible to hoist this inner work out to make this work but I want to show you the other way which is more the way a specialized if method would work (or the way we could memoize values in an array).

const count = S.data(6);
const parentNode = document.createElement('div')

let dispose;
S.cleanup(() => dispose && dispose());
S( prev => {
  const result = count() > 5;
  if (result === prev) return prev;
  dispose && dispose();
  if (result) {
    S.root(disposer => {
       dispose = disposer;
       parentNode.textContent = '';
       const node = document.createElement('div');
       S(() => node.textContent = 'Tracked ' + count() + ' times.');
       parentNode.appendChild(node);
    });
  } else {
    parentNode.textContent = 'Too Low';
  }
  return result;
});

Ok a lot more here. See the problem is to prevent repeat work we need to exit early if value of the condition hasn't changed. The problem is the nested computations would be released on re-run and the nested view wouldn't update anyway. By placing them in their own root the outer computation is no longer responsible for disposing them. However, now you are. So you need to make sure you dispose on re-run where applicable or if the outside context would ever re-run.

Truthfully though unless you are writing like array mapping control flow or trying to memoize nested values you should almost never need to make the roots yourself.

@mrjjwright
Copy link
Author

mrjjwright commented Mar 10, 2019

@adamhaile addressing your response first (holy cow, having a response from both of you so fast is just wonderful)

Yes, that sounds like exactly the issues with which I am dealing, thank you for being intuitive enough to see into my question. I had a little signal machine function (a function that returned some new signals plus computations in a plain JS object), and it was computed in an S.on. This return object I used to create other computations (in another tick of the S.js rootClock) and those computations worked fine but the computations created in the original object in the function were not firing unless I put setTimeout or S.root around my function. Anyway, that is probably a tad hard to follow, but yes that is exactly right.

The takeaway I am getting from you Adam is that in these cases it is often necessary to make sure that these types of functions return a computation based on the new state signals, and that returned S.js computation node should then update properly when the new signals change.

Ok, on to @ryansolid's comment.

@adamhaile
Copy link
Owner

adamhaile commented Mar 10, 2019

Ryan, interesting example. An equivalent of your first case that does it all in JSX would be:

const count = S.data(6);
const parentNode = <div>{count() > 5 ? <div>Tracked {count()} times</div> : 'Too Low'}</div>;

As you say, that would re-create the inner <div> each time count() changed.

You could fix some of that by doing:

const count = S.data(6);
const childNode = <div>Tracked {count()} times</div>;
const parentNode = <div>{count() > 5 ? childNode : 'Too Low'}</div>;

That's still not perfect: childNode gets updated even when count() <= 5, and it gets re-inserted each time count() changes while greater than 5. [edited to add: it's not really re-inserted, content() will see that it's already there and perform no dom ops, but still, it would be performing that check every time.]

You could get the efficiency right at the expense of synchronicity by piping the test into an S.value():

const count = S.data(6);
const isBigEnough = S.value(true);
S(() => isBigEnough(count() > 5));
const parentNode = <div>{isBigEnough() ? <div>Tracked {count()} times</div> : 'Too Low'}</div>;

Subclocks would fix the synchronicity issue (though again, they're not quite ready for production):

const count = S.data(6);
const isBigEnough = S.subclock(() => {
    const isBigEnough = S.value(true);
    S(() => isBigEnough(count() > 5));
    return isBigEnough;
});
const parentNode = <div>{isBigEnough() ? <div>Tracked {count()} times</div> : 'Too Low'}</div>;

If there were an S utility, called, say, S.expr(), that does the inner work there, you could simplify it all to just:

const count = S.data(6);
const isBigEnough = S.expr(() => count() > 5);
const parentNode = <div>{isBigEnough() ? <div>Tracked {count()} times</div> : 'Too Low'}</div>;

That would get maximum efficiency :).

@mrjjwright
Copy link
Author

Wow @ryansolid you just answered a question I had posed in your jsx babel repo, regarding when it is necessary and why to do manual disposing with S.js dom reconcilation, and as usually you explained things extremely well. This is something I really need to be aware of. I am using Surplus's runtime (brought directly into my app by copying the source :) ) and I set up content and insert renders very successfully over the last few days. I will keep this in mind as I look into the subtleties of how my little components render and re-render after I read and re-read your answer several times. :)

@ryansolid
Copy link

Thanks Adam. I chose that example because I figured it was the simplest one I could think of that could use nested roots realizing there were other solutions. In generalizing the solution as a utility function in the same way SArray map methods, is the nested root approach reasonable?

@ryansolid
Copy link

ryansolid commented Mar 10, 2019

@mrjjwright I realize I may have prematurely led you down a rabbit hole in that other issue. I had been working so much on my own library I wasn't thinking about how you'd be likely using S-array. I can explain why you don't see S.root in the Surplus code and you do in the babel plugin. It's because at the point of insert or content in Surplus the memoized lookup has already happened. Those methods are just taking a node array and diffing it to figure out the least number of changes and applying it. The nested S.root in this scenario would happen in the S-array map methods that do the actual lookup and data to DOM transformation.

With Surplus on update there are 3 steps.

  1. You update the data which triggers the computation in the S-array map function to fire.
  2. The map function finds or creates mapped nodes registering disposables for each new root it creates. It sorts and disposes the nodes so that the resulting node list is what you wish the final DOM state to be.
  3. The resulting node array is passed into insert or content and uses the algorithm to make the least changes to the DOM to reflect the list.

The difference with the babel plugin is I combined step 2 and 3 into a single pass. The each binding is essentially S-array mapSample + insert/content. I do something similar for my when binding which is similar to the psuedo code I posted above. So if you use S-array it is possible you don't need to worry about handling this yourself as it will handle lists and the if case can be handled by hoisting or wrapping the execution of the computation in nested computation to prevent the declaration from being in the same context as the execution like Adam suggested.

Take a look at mapS and mapSample if you want another example of how the nested roots work.

@mrjjwright
Copy link
Author

mrjjwright commented Mar 10, 2019

@ryansolid I notice that S-array is not a dependency (or peer dependency) for Surplus so I am assuming that a Surplus user must use mapSample in certain cases that are along the lines of this discussion and I see that this is indeed done by @adamhaile e.g. in the surplus-todomvc example here:

https://github.com/adamhaile/surplus-todomvc/blob/master/src/views.tsx#L25

I also see that in the source for mapSample there is an internal object called mapped that contains S.roots which matches what you are saying.

I also understand that you wanted to combine the passes through the array in your babel plugin, probably for performance and correctness reasons and then mimic what is done to some extent in mapSample.

I also see that there is another case covered by your when that is very similar that I need to study a bit more.

So I see the general outlines of what you are saying, thank you so much for the clear explanation and I will keep studying to understand more.

I figured out my original problem. It's really the fundamentals that continue to trip me up, no big surprise since that is true with most anything in life. In my app, when the user presses a key, I set the KeyboardEvent on a signal and then run a S.on computation that tests the key and if a certain key is pressed a new UI object is pushed into my app state (in an SArray) with new signals and computation. So in each subsequent press of the keyboard, and hence on the updates to this S.on computation, where a new path through the key handler computation was being followed, all the computations I was creating in this function were getting disposed, while all the signals were getting kept, since there were stored elsewhere. Also other computations created in other places of my app that used the new signals would continue to work, which confused me, just not the computations created within the S.on computation. They continued to work because their lifetimes were tied to other computations that were still alive.

The big takeaway for me is to continue to remember that dependent computations created in one update of a computation will be disposed in the next. Any objects created within a particular path of a computation could go away at anytime and so I need to make sure the paths through that computation make a ton of sense for the networks I am building of dependent parent/child signal/computations. The lifetime of computations is in practice nearly automatic, and hence different than the lifetime of signals (as you pointed out, which helped trigger my understanding). In most cases signals will get stored/referenced in the user's own app, but computations lifetimes are much more handled internally by the S.js internal ComputationNode, Log, RootClock, event, update, disposer mechanisms.

Feel free to state it better if somebody sees any flaws in my understanding. @adamhaile and @ryansolid, again thank you so much. I am absolutely in love with this stuff and because both of your code is written so minimally and clearly (unlike the bigger libs of mobx/vue/rxjs), I hopefully have a chance at actually creating bug free code around this that still has great performance.

@adamhaile
Copy link
Owner

run a S.on computation that tests the key and if a certain key is pressed a new UI object is pushed into my app state (in an SArray) with new signals and computation.

Yeah, ok, that's a prime case of where S.root() is going to be involve somewhere, because you want computations to live beyond the update cycle of the computation that created them. I've used two solutions for this scenario in apps:

  1. Let SArray manage it for me. Instead of creating the new object directly, I push its constructor parameters onto an inits SArray. That array is mapSample'd to run the actual constructor. That way the map owns all computations created, and keeps them alive. If I want the object to be disposed, I remove its constructor params from the inits array and it is removed from the mapping.
const 
   inits = SArray[]),
   objects = inits.map(params => new SomeObject(params));

S.on(someEvent, () => inits.push(newObjectConstructorParams));

// ... later, when it's time to dispose it
inits.remove(newObjectConstructorParams);
  1. Use S.root(), get a dispose function, which is passed to the new object. When we want to get rid of it, we call that dispose method. Usually I still want to push this object onto some SArray so I can enumerate all such objects.
const objects = SArray([]);

class SomeObject {
    constructor(params, dispose) {
        ....
        this._dispose = dispose;
        objects.push(this);
    }

    dispose() {
        this._dispose();
        objects.remove(this);
    }
}

S.on(someEvent, () => S.root(dispose => 
    new SomeObject(params, dispose)
));

// ... later 
object.dispose();

@mrjjwright
Copy link
Author

mrjjwright commented Mar 11, 2019

@adamhaile I did almost exactly # 2 of your solutions on my own yesterday but just hadn't understood why it was working and needed until this morning, when some comments by @ryansolid tripped my understanding. The first solution is super cool and since my SArray holds a "stack" of UI objects that come and go, it might be perfect for me! It was such a relief to understand all this today and once I did I am really flying, app is working great again, so thanks to you all again. 🚀

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

3 participants