[Streams 🌊] Improve definition narrowing and reduntant requests

[tonyghiani]

📓 Summary

These changes lift the check against the definition existence and narrows its value for the react context consumers.

It also fixes reduntant requests for the AI connectors used for the grok parsing suggestions.

@flash1293 I'd expect to use the AI capabilities across more places for the enrichment experience, we should probably lift the AI capabilities as part of the page initialization at a certain point, although it's not needed yet 👌

[elasticmachine]

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

[flash1293]

DIdn't check in depth yet, but this might be related to #215517 ? Maybe not. My PR is improving the loading of the definition, maybe it's upstream from your changes. Will check later.

[tonyghiani]

Didn't check in depth yet, but this might be related to #215517 ?

Does not seems related, this work only help to guarantee the definition already exists inside the StreamDetailProvider that fetches the definition, lifting all the if (!definition) return null kind of checks we have around the codebase.

The fix for reduntant requests for connectors is also unrelated.

[flash1293]

Cool - let's get rid of all the unnecessary loading :)

[flash1293]

Please don't merge yet

[tonyghiani]

Please don't merge yet

@flash1293 is there any specific blocker on this?

[flash1293]

I like how this makes the code cleaner, but I don't like how it changes the way the page is rendered progressively.

This is the current state:

This is the state after your change:

It basically renders a blank page for a bit, while the previous version shows an empty skeleton that's then filled in (we don't show one of the panels right away, we should probably fix that).

Not sure about the easiest way to handle it, but I'm not 100% convinced this change is a net positive.

[tonyghiani]

It basically renders a blank page for a bit, while the previous version shows an empty skeleton that's then filled in (we don't show one of the panels right away, we should probably fix that).

That's a fair concern and we should probably address it, IMO a simple page skeleton or loader while loading would be enough instead of rendering the blank page, mostly because it prevents us from showing a lot of empty components around or multiple loading states. I can take a look back at this, I'll ping you later with some changes.

[flash1293]

A proper skeleton would be great 👍

[flash1293]

I think a centered loader would probably be fine too - in a lot of cases this request is fairly fast. I also don't want us to lose too much time on this.

[dgieselaar]

@tonyghiani perhaps just wrap in a waitUntil() function that renders a loading spinner until a certain condition is resolved? I figure we can use it many places. e.g.:

import { z } from 'zod';
import React from 'react';

function waitUntil<TProps extends Record<string, any>, U extends Partial<TProps>>(
  schema: z.Schema<U> | (( props:TProps ) => U | undefined),
  next: (props: TProps & U) => React.ReactNode
): React.FC<TProps> {
  if (typeof schema === 'function') {
    return ( props:TProps ) => {
      const validated = schema(props);
      if (validated) {
        return next({
          ...props,
          ...validated
        });
      }
      return <EuiLoadingSpinner/>;
    }
  }
  return (props: TProps) => {
    const result = schema.safeParse(props);
    if (!result.success) {
      return null;
    }
    return <>{next({ ...props, ...result.data })}</>;
  };
}

export const MyComponent:React.FC<{ maybe?:string }> = waitUntil(
  ( props ) => {
    if (props.maybe) {
      return {
        ...props,
        maybe: props.maybe!
      };
    }
    return undefined;
  },
  next => {
    return null;
  }
)

[tonyghiani]

@flash1293 I added a simple spinner as a full page skeleton might be overkilling and it would appear for a very short time, the spinner appears on the first load but not while the definition is revalidated to avoid page unmounts unnecessarily, sort of SWR approach.
I noticed an issue that reloads the definition when going to the management tab, but I think you addressed it already in #215517. This should be ready for another check 👌

@dgieselaar thanks for the suggestion, that would make sense if case we have more conditional renders waiting for data-fetching, in this case a simple if statement does the job and the HOC + zod seems a bit too much until we need it, worth keeping an eye on this anyway for upcoming work 👌

[flash1293]

LGTM

[elasticmachine]

💚 Build Succeeded

• Buildkite Build
• Commit: ed65130
• Storybooks Preview

Metrics [docs]

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.9KB	392.7KB	-252.0B

History

• 💔 Build #288931 failed dca8a48
• 💚 Build #288460 succeeded 4f99962
• 💔 Build #288349 failed 35e57b0
• 💚 Build #286991 succeeded cdd40c8

[kibanamachine]

Starting backport for target branches: 8.x

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

[kibanamachine]

💔 All backports failed

Status	Branch	Result
❌	8.x	Backport failed because of merge conflicts You might need to backport the following PRs to 8.x: - 🌊 Streams: Selectors for derived samples (#213638)

Manual backport

To create the backport manually run:

node scripts/backport --pr 215897

Questions ?

Please refer to the Backport tool documentation

[kibanamachine]

Starting backport for target branches: 8.x

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

[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