🌊 Streams: Selectors for derived samples

[flash1293]

Simplified massively from first state and just plugging in reselect in places where that's suitable (here to calculate the currently relevant sample documents).

Also does a drive-by layout fix.

Introduces a new xstate helper for derived data.

In most cases, the actor and state machine model of xstate is great, but for derived data using pure functions, the semantics of the useMemo hook with defined dependencies is often easier to understand and eliminates the risk of forgetting to update the derived data correctly in some cases.

It's about using the right tool for the right job - you don't need to choose between the dependency list of useMemo and the actor model of xstate, you can use what fits the case, without compromising performance.

This is the API:

const myActorContext = withMemoizedSelectors(
  createActorContext(myMachine),
  {
    derivedView: createSelector(
      [
        (ctx: MyContextType) => {
          return ctx.dependency1;
        },
        (ctx: MyContextType) =>
          ctx.dependency2,
      ],
      (dependency1, dependency2) => {
        return // expensive calculation only running when necessary
      }
    ),
  },
  (context) => (context.subMachine ? [context.subMachine] : []) // optional subscribe to changes of submachines as well
);


// in react use useMemoizedSelector hook
// this will cause the component to rerender if the selector is returning a new value
myActorContext.useMemoizedSelector('derivedView')

This is using reselect to declare the dependencies similar to a react useMemo hook - the actual selector will only run if the dependencies change, leading to similar semantics as useMemo, with the additional benefit that if the value is used in multiple places, it's still just calculated once. The component calling withMemoizedSelectors only re-renders if the value returned by the selector changes. The selector itself only re-runs if one of the declared dependencies changes.

Everything is type-safe by capturing the types of the reselect selector object via inferred type param and using it in the useMemoizedSelector type.

[elasticmachine]

Pinging @elastic/obs-ux-logs-team (Team:obs-ux-logs)

[tonyghiani]

When we first talked about this I liked the idea of using reselect for this purpose, but I had something much simpler in mind. Maybe you went throught the same mental steps, so please correct me if I'm missing something.

Using reselect, results are cached unless any of the specified dependencies change.
With that in mind, we shouldn't need an additional context wrapping around the machine provider, but in general the whole abstraction could be removed.

The approach I thought you had in mind was simpler, purely based on defining selectors and use them along with useSelector from xstate. For instance, in the first case to derive previewDocuments, we could just have

// ./state_management/simulation_state_machine/selectors.ts
const EMPTY_ARRAY: [] = []

export const selectPreviewDocuments = createSelector(
  [
    (snapshot: SimulationActorSnapshot) => snapshot.context.samples,
    (snapshot: SimulationActorSnapshot) => snapshot.context.previewDocsFilter,
    (snapshot: SimulationActorSnapshot) => snapshot.context.simulation?.documents,
  ],
  (samples, previewDocsFilter, documents) => {
    return (
      (previewDocsFilter && documents
        ? filterSimulationDocuments(documents, previewDocsFilter)
        : samples) || EMPTY_ARRAY
    );
  }
);

// On any component needing the preview documents
const previewDocuments = useSimulatorSelector(selectPreviewDocuments);

This wouldn't need to create an additional layer on top of the machine with all the related subscription, but we can derive the value we need directly on usage site, relying on the stronger selection mechanism by reselect.
For the use cases we have now, I believe it would make runtime computation cheaper and DX faster given we need to only define the selector.

I didn't try all of this, let me know if you want me to give it a shot or you tried this already 👌

[flash1293]

@tonyghiani Good call - I didn't test this, I will try. I think it won't work as-is because reselect is also memoizing on the argument and it's probably stable? Basically it's missing this line I had to sneak in there: https://github.com/elastic/kibana/pull/213638/files#diff-6323649842f84a1364b2e0be97b01fe708826d3ea4759452cf02a54fc060a5a3R84

But that's most likely fixable with a very lightweight wrapper - great call for sure! Definitely simplifies a bit the setup and it's nice that it only calculates the derived view if there is actually a consumer currently rendered somewhere.

I will look into this and update the PR.

[tonyghiani]

I think it won't work as-is because reselect is also memoizing on the argument and it's probably stable?

This is right, in that case might be worth to not use the stable reference of the snapshot but directly pass the context as the selector input:

// ./state_management/simulation_state_machine/selectors.ts
const EMPTY_ARRAY: [] = []

export const selectPreviewDocuments = createSelector(
  [
    (context: SimulationContext) => context.samples,
    (context: SimulationContext) => context.previewDocsFilter,
    (context: SimulationContext) => context.simulation?.documents,
  ],
  (samples, previewDocsFilter, documents) => {
    return (
      (previewDocsFilter && documents
        ? filterSimulationDocuments(documents, previewDocsFilter)
        : samples) || EMPTY_ARRAY
    );
  }
);

// On any component needing the preview documents
const previewDocuments = useSimulatorSelector(snapshot => selectPreviewDocuments(snapshot.context));

This should work as the context object will update once anything is assigned to the itself from on a transition

[flash1293]

const previewDocuments = useSimulatorSelector(snapshot => selectPreviewDocuments(snapshot.context));

Now that you put it there - it won't work in practice because a change in a sub machine is

• Not creating a new context reference for the top level machine (so memoization with cause it to skip)
• Not triggering a re-evaluation of useSimulatorSelector in the first place (so it will not even be called)

[tonyghiani]

Now that you put it there - it won't work in practice because a change in a sub machine is

• Not creating a new context reference for the top level machine (so memoization with cause it to skip)

I forgot to add a step, my bad!
In this work you moved in the previewDocuments derived value from the simulation actor to the StreamEnrichmentContext.

What I mean is that the selector could act directly against the simulation actor, using the useSimulatorSelector directly. In this case the hook should trigger a re-evaluation when its internal state changes, which would be enough to recompute the the previewDocuments.
I didn't try it as I said, but the useSimulatorSelector defined here could make the trick.

That's already used in some places, not sure if would fit well with the selector but it's worth trying 😄

[flash1293]

Oh, right, that should work. I'll put it together like this, thanks 👍

[flash1293]

@tonyghiani ready for another look

[tonyghiani]

Love this change, using reselect makes it much cleaner and we don't need to make it more complex with wrappers on top of the machine 👌

[kibanamachine]

Starting backport for target branches: 8.x

https://github.com/elastic/kibana/actions/runs/14192518199

[elasticmachine]

💛 Build succeeded, but was flaky

• Buildkite Build
• Commit: f13bfd0

Failed CI Steps

• Jest Tests #14
• Jest Tests #20

Test Failures

• [job] [logs] Jest Tests #14 / should render empty component if missing workflow_user value
• [job] [logs] Jest Tests #20 / Templates renders empty templates correctly

Metrics [docs]

Module Count

Fewer modules leads to a faster build time

id	before	after	diff
streamsApp	424	425	+1

Async chunks

Total size of all lazy-loaded chunks that will be downloaded as the user navigates the app

id	before	after	diff
streamsApp	392.1KB	392.1KB	-62.0B

Page load bundle

Size of the bundles that are downloaded on every page load. Target size is below 100kb

id	before	after	diff
streamsApp	9.6KB	9.6KB	+60.0B

History

• 💛 Build #288652 was flaky 78b5eaa
• 💔 Build #288350 failed b4fa735
• 💚 Build #288011 succeeded e7b632e
• 💚 Build #286488 succeeded 3548e96
• 💛 Build #286030 was flaky db1702a
• 💚 Build #285244 succeeded 93c8935

[kibanamachine]

💚 All backports created successfully

Status	Branch	Result
✅	8.x

Note: Successful backport PRs will be merged automatically after passing CI.

Questions ?

Please refer to the Backport tool documentation