-
Notifications
You must be signed in to change notification settings - Fork 9
iframe view generation
#150
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
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
added comments to explain context. i think i'll go ahead and do those changes.
typescript/packages/lookslike-high-level/src/components/iframe-view.ts
Outdated
Show resolved
Hide resolved
typescript/packages/lookslike-high-level/src/recipes/iframeExample.ts
Outdated
Show resolved
Hide resolved
typescript/packages/lookslike-high-level/src/recipes/iframeExample.ts
Outdated
Show resolved
Hide resolved
| @customElement('common-iframe') | ||
| export class CommonIframe extends LitElement { | ||
| @property({ type: String }) src = ''; | ||
| @property({ type: Object }) context?: CellProxy<any>; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
ok, a little issue here is that the UI framework currently listens to that cell already and then issues property updates to the lit element if anything in that cell changes. it doesn't follow cells however. (meta: the ui framework precedes the proxy stuff, so probably it's the ui framework that needs some changes)
we can do the following:
- exploit that it only listens to the first level by sending data as
.context=${{context: data}}(i.e. an object with a data property that then points to the actual cell. - now context.context is going to be a Cell. That's now a "simple cell" in cell.ts, which still doesn't automatically follow references. so we can do three things:
- manually "crawl" the cell and add listeners to all levels. add a guard against infinite loops by pointing to existing objects when retrieved. a bit complicated, but safe. eventually we want to add a schema that limits how deep we go.
- convert to a CellProxy and deep copy it. We'd have to add a method to the simple cell for this, easy. Beware: This can cause infinite loops, since behind the proxy is a generic graph, not a tree.
- Present a simple 'get'/'sink'/'write' interface to the iframe and let the iframe manages how deep it wants to go. that's the most work, but also mirrors how we'd want the JS inside our sandboxes to work. we would have to write a library that runs inside the iframe, so that the LLM writes against a simpler API (and unfortunately that one is then different than the one in the sandbox, because this is long-term reactive vs the synchronous style in the sandbox).
i'd go with 2 for expedience and then see.
| // TODO(ben): should be subscribing to just the cell, not the whole context | ||
| // but doing .getAsProxy([key]) or [key] both seem to be lacking .sink()? | ||
| const unsub = this.context.sink(() => { | ||
| this.notifySubscribers(key, this.context.get()[key]); | ||
| }); | ||
| this.subscriptions.set(key, unsub); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@seefeldb not sure what I'm missing here, this code works because it's triggering for any change but I wanted to scope to just a single key... Is it because the value is a primitive (number)? Am I just holding it wrong?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
/me really needs to write that README :)
.getAsProxy() turns it into a proxy that just represents the data, but also traverses cell references, since our data can be a graph.
this.context.key(key).sink() would do what you want, but it won't subscribe to changes in referenced cells.
what we can do is call .getAsProxy([key], log) where log is a ReactivityLog. And then do the deep copy. Then log contains all the cells actually read.
But at this point, we can also use the actual scheduler to push changes. Let me do this quickly, might need a few small tweaks.
Minimum viable generative UI.
Some issues with reactivity and sharing the state with the iframe context (see comments in code). I'm using a basic
read/writepostMessage API.