feat: Add support for object returns and nested JSX elements #28

EvanBoyle · 2024-12-24T22:19:23Z

Add two features to streamline workflow authorship:

Support for returning objects instead of just Element Element[] and string
Add the ability to embed Elements inside of objects

Direct Object Return

Components can now return plain objects directly instead of only JSX elements
Eliminates the need for wrapper components when combining multiple outputs
Example: return { summary, commentAnalysis } instead of needing a <CombineOutput> component

Nested JSX Resolution in Objects

The execute function now recursively resolves JSX elements within returned objects
Enables natural composition where object properties can be JSX elements
Example:

     return {
       analyses: stories.map(story => ({
         summary: <PostSummarizer story={story} />,
         commentAnalysis: <CommentsAnalyzer comments={story.comments} />
       }))
     }

Together both of these changes elimintate the need for wrapper components, boilerplate, and reduces the awkwardness of having to deal with weakly typed tuples in fanout and parallel operations.

jmoseley · 2024-12-25T05:05:54Z

I think this makes sense functionally, and is necessary to work around the tediousness required for returning multiple values down the tree. But it gets closer and closer to just nested function calls.

When someone asks why they should return a JSX component instead of returning the value from a function call, do we have a really good answer? I'm not sure answers like "you get caching" or "that's how the framework works" hold enough water.

jmoseley · 2024-12-25T05:08:06Z

src/jsx-runtime.ts

@@ -31,30 +32,39 @@ export const jsx = <
  component: (props: TProps) => MaybePromise<TOutput>,
  props: TProps | null,
  children?: (output: TOutput) => MaybePromise<JSX.Element | JSX.Element[]>,
-): Promise<TOutput | TOutput[]> => {
+): Promise<Awaited<TOutput> | Awaited<TOutput>[]> => {


TOutput is not a PromiseLike, we don't want Awaited here I don't think (even if the compiler really thinks we do).

But the component could return a direct value or a promise, no?

Something I had to solve to get this to work is traversing every element since it can contain:

a plain value

an array or object of promises or elements

And then making sure that every promise is awaited, and that every element is evaluated recursively until every returned value is plain. Though very possible I am missing something here.

But the component returns MaybePromise<TOutput>, so I believe that it should already be unwrapped.

EvanBoyle · 2024-12-25T05:46:53Z

But it gets closer and closer to just nested function calls. ... do we have a really good answer?

@jmoseley this is a great question. JSX components get us two things automatically:

The child composition pattern: data flow through the LLM pipeline is clear - each step receives plain values and passes them to the next step through a simple child function pattern. Raw functions mean that async handling and error management are manual (or that the user has to remember to call a special function wrapper), GenSX components automatically handle promise resolution and we can extend the framework around it to automatically handle retries, parallel execution, and error boundaries. The framework can handle tracking dependencies, figuring out which components can be executed in parallel, and general workflow optimization which is harder if everything is just a function.
It makes sharing and composing LLM operations natural. While you could build similar patterns with functions, components provide a clear standard.

A major goal of this PR is to make it much easier to share components. The most common case for LLM workflows by far is simple chaining. A -> B -> C -> final output. Cases where you have fork join patterns, reflection, agents, etc are still important but they are not the 80% case.

This change is focused on making it easier for the 20% use case to use components in whatever shape you need them in.

Think about the world where there is a critical mass of components. It needs to be easy to massage them into whatever output shape you need them in. This change enables that. Standardized components create an ecosystem. Shape flexibility enables component reuse. Both need to be true for a marketplace of LLM tools to emerge.

jmoseley · 2024-12-30T17:23:42Z

src/component.ts

  }
+
  if (fn.name) {
    Object.defineProperty(WorkflowFunction, "name", {
      value: `WorkflowFunction[${fn.name}]`,


Totally unrelated to this change, but I wonder if we should change the name of this to GsxComponent.

We can if we have a good reason.

We should work backwards from what we want the examples code to look like. gsx.Component feels good IMO, users can always import { Component as GsxComponent .. if they want, but I'm not married to the current convention either.

oh sorry, I meant specifically this string (and the name of the WorkflowFunction wrapper). So if the user does console.log they get something specific to gsx and not generic like WorkflowFunction[MyFunction]

jmoseley · 2024-12-30T17:25:42Z

src/index.ts

+export type { Element };
+
+// Collect component for parallel execution with named outputs
+export async function Collect<T extends Record<string, unknown>>(


Can we move this to its own file? Let's try to keep index.ts as just a barrel file.

I think this code gets deleted in a later PR. It is no longer needed.

jmoseley · 2024-12-30T17:26:25Z

src/index.ts

+      /* eslint-disable @typescript-eslint/no-unsafe-assignment */
+      /* eslint-disable @typescript-eslint/no-explicit-any */
+      /* eslint-disable @typescript-eslint/no-unsafe-member-access */
+      /* eslint-disable @typescript-eslint/no-unsafe-argument */


You can move these to the top of the (collect.ts) file, all this JSX code is effectively untyped, so no need to worry much about linting.

jmoseley · 2024-12-30T17:33:10Z

src/resolve.ts

+import { ComponentProps, ExecutableValue, WorkflowComponent } from "./types";
+
+// Helper to check if something is a JSX element
+/* eslint-disable @typescript-eslint/no-unsafe-return */


Might as well just move these to the top of the file I think.

jmoseley · 2024-12-30T17:36:28Z

src/resolve.ts

+    "type" in element &&
+    "props" in element &&
+    typeof (element as any).type === "function" &&


We don't use the .type pattern to store a reference to the original function (I think it comes from React).

jmoseley · 2024-12-30T17:37:24Z

src/component.ts

+  // Mark as workflow component and JSX element type
+  const component = WorkflowFunction as WorkflowComponent<P, O>;
+  component.isWorkflowComponent = true;


nit, let's make it clear this is an internal thing:

Suggested change

// Mark as workflow component and JSX element type

const component = WorkflowFunction as WorkflowComponent<P, O>;

component.isWorkflowComponent = true;

// Mark as workflow component and JSX element type

const component = WorkflowFunction as WorkflowComponent<P, O>;

component.__isGsxComponent = true;

jmoseley · 2024-12-30T17:45:56Z

src/resolve.ts

+
+  // Handle JSX elements
+  if (isJSXElement(value)) {
+    const componentResult = await value.type(value.props);


I can't figure out where .type is being assigned. I've noticed that claude likes to write this sort of code, I think it's inspired by the React codebase (which uses this pattern under the hood).

And I'm not sure we need to follow the same pattern, I've avoided the extra complexity so far.

Yeah, I thought it was being assigned at the framework layer, but it looks like that must've been lost at some intermediate step.

I'm surprised that it works at all with type being undefined, but it looks like isJSXElement always returns false, but resolveDeep ends up doing the right thing.

We should talk through the right refactor here. Maybe we can just remove most of this code.

EvanBoyle · 2024-12-30T22:34:00Z

closing in favor of #36

add support for object returns and object JSX values

3857ead

github-actions bot added the area/source label Dec 24, 2024

EvanBoyle added 4 commits December 24, 2024 16:21

recursive execution

6e38b85

refactor to use shared resolve function

f7a9b72

remove logging

411bb15

linting

2d05139

EvanBoyle marked this pull request as ready for review December 25, 2024 03:59

EvanBoyle requested a review from jmoseley December 25, 2024 03:59

jmoseley reviewed Dec 25, 2024

View reviewed changes

jmoseley reviewed Dec 30, 2024

View reviewed changes

EvanBoyle closed this Dec 30, 2024

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

feat: Add support for object returns and nested JSX elements #28

feat: Add support for object returns and nested JSX elements #28

EvanBoyle commented Dec 24, 2024

jmoseley commented Dec 25, 2024

jmoseley Dec 25, 2024

EvanBoyle Dec 25, 2024

jmoseley Dec 30, 2024

EvanBoyle commented Dec 25, 2024

jmoseley Dec 30, 2024

EvanBoyle Dec 30, 2024

jmoseley Dec 30, 2024

jmoseley Dec 30, 2024

EvanBoyle Dec 30, 2024

jmoseley Dec 30, 2024

EvanBoyle Dec 30, 2024

jmoseley Dec 30, 2024

jmoseley Dec 30, 2024 •

edited

Loading

jmoseley Dec 30, 2024

jmoseley Dec 30, 2024

jmoseley Dec 30, 2024

EvanBoyle Dec 30, 2024

EvanBoyle commented Dec 30, 2024

feat: Add support for object returns and nested JSX elements #28

feat: Add support for object returns and nested JSX elements #28

Conversation

EvanBoyle commented Dec 24, 2024

Direct Object Return

Nested JSX Resolution in Objects

jmoseley commented Dec 25, 2024

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

EvanBoyle commented Dec 25, 2024

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

jmoseley Dec 30, 2024 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

EvanBoyle commented Dec 30, 2024

jmoseley Dec 30, 2024 •

edited

Loading