BrightspaceHypermediaComponents / siren-sdk

This contains tools to help develop and use siren entities.
Apache License 2.0
2 stars 2 forks source link

State management of children #57

Open catwomey opened 5 years ago

catwomey commented 5 years ago

Problem

Managing chains of related items results in long callback chains as seen in https://github.com/BrightspaceHypermediaComponents/enrollments/pull/144/files#diff-bc6e475d126bd0aca44f8e5152cde3f8R404-R422

Managing the state of related items is proving to be difficult and we need to simplify it for consumers.

Proposed Solutions

TBD

AllanKerr commented 5 years ago

The primary complexity that arises here isn't from handling the nested onChange handlers although it is still ugly. When entities change, the onChange handlers contain most of that state implicitly because they trigger cascading change handlers.

In the example below, moduleKey uniquely identifies the module and we can easily manage updates with a flat dictionary.

 organization.onSequenceChange(rootSequence => {

    rootSequence.onSubSequencesChange(subSequence => {

        subSequence.onSequencedActivityChange(sequencedActivity => {

            sequencedActivity.onOrganizationChange(organizationEntity => {

                const moduleKey = `${subSequence.index()}-${sequencedActivity.index()}`;

            });
        });
    });
});

This solution falls short because, if the onSequenceChange handler is called, we can't guarantee that the number of sub-sequences hasn't changed. If there are fewer sub-sequences then there will be no onSubSequencesChange handler for the sub-sequence that is no longer present and our flat dictionary will now be storing a stale moduleKey.

As for what the best solution is, I'm not entirely sure but in its current state we do need some sort of tree to manage the state to prune the sub-tree if the parent changes. It feels like this issue in particular is a result of onChange handlers that do fan-out because we can't guarantee the number of times those will be called.

catwomey commented 5 years ago

Interesting, apologies for missing the core issue.

Something I was toying with in another component, is if the root changes you essentially dispose all children and go through the entire flow again essentially re initializing the component. This ensures that you have the freshest data based on the root. Because if the root changed there's a good chance the rest of your tree is completely different.

To ensure you aren't churning in your display, diffing your component state with your siren state could be used. This might not work for complex use cases like the sequences domain though.