-
Notifications
You must be signed in to change notification settings - Fork 0
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
Use Proxy
to allow for arbitrary property/array access
#96
base: liam/future-types
Are you sure you want to change the base?
Conversation
// prop.type = string | ||
// prop.ref = None | ||
// prop_return_class = Future<string> | ||
// use_proxy = False |
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.
I'm planning to remove this information from the codegen output, but I've been using it to debug for now.
src/ProxiedFuture.ts
Outdated
return () => { | ||
const utarget = unproxy(target); | ||
const id = futureId(utarget); | ||
futureTable[id] = utarget; // store in lookup table | ||
return id; |
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.
I may also add a runtime check here to assert that target
in this context is a Future
value so that other arbitrary objects cannot be used with bracket access on proxied Future
values
import { Future, Trace } from "substrate/Future"; | ||
|
||
// Only Futures that resolve to string | number are legal property accessors | ||
type FutureTable = Record<string, Future<string | number | unknown>>; |
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.
type FutureTable = Record<string, Future<string | number | unknown>>; | |
type FutureTable = Record<string, Future<string | number>>; |
|
||
// @ts-ignore (access protected prop: _directive) | ||
const trace = target._directive.next(nextProp); | ||
if (!(trace instanceof Trace)) throw "something's not right."; |
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.
For now, I think that we're only creating Future
values with a Trace
directive, but I'm not sure that it would always be the case if we used proxies as return values from other things besides node output futures.
a022cd0
to
4f1685c
Compare
Warning
This branch is quite experimental and we're still working through it's implications.
This branch is a follow up to #93 and the goal is to attempt to allow for arbitrary property (and array indexing) access on
Future
values that are defined by the user.With this implementation, you can do the following:
In the example above,
Box
allows the user to define some arbitrary structure, so theFuture
values that are accessed from it will have the shape described by the input object.These
Future
objects are implemented using aProxy
and intercepts accessors to produce newFuture
values which are wrapped again in aProxy
allowing us to access further properties in these structures.Internally, we keep track of the accessors and use them to construct the
Trace
directive that describes to the server how to access the variable at some given accessor chain. When we acceptFuture
values as inputs to our various helper functions (insb
, likeconcat
,jq
, etc) we "unproxy" the potentially proxied value so that we can interact with the "target"'s actual properties instead of the "virtual" properties that are enabled through the proxy.Currently I've only implemented these proxied
Future
values on the generatedNode
Future
properties that return arbitrary objects. For example, forBoxOut.value
,IfOut.result
,ComputeJSONOut.json_object
(and other LLM nodes),Embedding.metadata
, etc. We might also want to return proxiedFuture
values from other functions too, likesb.jq
andsb.get
, and I am exploring that in another branch.Property access and Array indexing
This implementation supports accessing properties on objects using "dot notation" (eg.
object.property
) and bracket access (eg.object["property"]
). Additionally because these proxied objects may also represent arrays, the bracket access may also acceptnumber
values too (eg.object[123]
). A special use case also implemented here is to support the use ofFuture
values as accessors, which may be used with bracket access (eg.object[future]
).The JavaScript runtime will attempt to convert any value used with bracket indexing (on an object) into a
string | Symbol
. Because of this, the implementation here handles the case of "numeric" values andFuture
values as special cases.For "numeric" values (any "digit-only" value like
123
or"123"
) will be treated as array-indexing access and will serialize this accessor as agetitem
"op". This means that an object with the property"123"
may not work as expected, but this might be a reasonable tradeoff as properties do not often use digit-only property names.For
Future
values the SDK maintains a hidden lookup table forFuture
values that are used as accessors. When theFuture
is used in this context (as a side-effect of thetoPrimitive
conversion) we store the value in the lookup table and return a specially-formatted string key that the proxy can use to look up theFuture
and store this internally in the newFuture
as the next accessor.Known Limitations
object["123"]
- but instead be treated as array indexing access.any
, so the type of these objects currently isany
. I'm going to explore using type parameters in these cases that can transform user-defined types into types with wrappedFuture<T>
types and use type assertions to allow these values to "masquerade" as the provided type and be reflected in LSP toolingFuture
, but will be given no type error - however they will receive a nodedependency_error
with the message"Unexpected exception while resolving arguments for node"
Future
value (not proxied and cast toany
) the user will need to add a type assertion to avoid TypeScript warnings that"Future<X> cannot be used as an index type."
. For example,let newProxiedFuture = proxiedFuture.x.y.z[notProxiedFuture as any]
. This doesn't apply to proxied-futures because these are currently cast toany
already.Outstanding Issues
The server side "implicit graph" is not handling a case where
Future
values from the same node are combinedI've encountered an issue when building some simple test examples where using two
Future
values from the same node together result in the node being dependent on itself causing the node not to run because it's dependencies are not resolved.For example,