-
Notifications
You must be signed in to change notification settings - Fork 16
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
Add initial views API #978
base: main
Are you sure you want to change the base?
Conversation
@@ -0,0 +1,67 @@ | |||
{ | |||
"name": "@osdk/e2e.sandbox.views.todo-widget", |
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.
note i haven't actually ran this yet, i mostly made it to make sure all the types are right. I will go after a proper template (+ vite plugin) in a follow up PR
// Unfortunately the context is statically defined so we can't use the generic type, hence the cast | ||
} as FoundryViewClientContext<ParameterConfig>} |
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.
if there's a way to do this without the cast i'm all ears 😬
if (window.top == null) { | ||
throw new Error("[FoundryViewClient] Must be run in an iframe"); | ||
} | ||
this.parentWindow = window.top; | ||
const metaTag = document.querySelector( | ||
`meta[name="${META_TAG_HOST_ORIGIN}"]`, | ||
); | ||
if (metaTag == null) { | ||
throw new Error( | ||
"[FoundryViewClient] Missing host origin meta tag " | ||
+ META_TAG_HOST_ORIGIN, | ||
); | ||
} |
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.
there's an argument to be made that maybe this shouldn't throw in development mode..? on the one hand, it's nice to be consistent in how the code runs whether its in production or not. on the other hand, it means you can't actually develop locally without using the developer mode that Foundry will provide for developing on these views
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.
Hard to say without thinking about what a disconnected dev/test setup would look like. We might want to provide a stubbed alternative to FoundryViewClient which mocks API calls. Otherwise you'd need a local mock HTML renderer which could provide this value anyway
ParameterValueType, | ||
StringParameterValue, | ||
TimestampParameterValue, | ||
} from "./parameters.js"; |
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.
Should we be exporting all of the array variants too? StringArrayValue
etc
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.
hm good question
|
||
export interface AsyncLoadedValue<V> { | ||
type: "loaded"; | ||
value: V | undefined; |
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 still want to spend more time thinking about the | undefined
s. If we support marking parameters as required we could treat undefined as a failed state, or a default value. One for later
window.removeEventListener("message", this.listener); | ||
} | ||
|
||
public sendMessage(message: ViewMessage<CONFIG>) { |
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 think it would make sense to have a event-type specific public methods, i.e. emitEvent(event: Omit<ViewEmitEventMessage, "type">)
.
If we are auto-sending the ready event it never really makes sense for the dev to resend it. And when ViewMessage
is extended to fetch-intercept events we should probably keep those off the API.
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.
ah, makes sense. i can probably narrow the types so we only explicitly have the ones we want devs to call from the client
>(initialValues); | ||
useEffect(() => { | ||
client.subscribe((event) => { | ||
if (event.data.type === "host.update-parameters") { |
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.
isHostParametersUpdatedMessage
?
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.
aesthetically i kind of like it as-is since typescript knows how to narrow the type/don't need to add more functions in the mix. i think we can still export typeguards if people want to use it though
This adds 4 new packages:
@osdk/views-api.unstable
: Primarily types that define the contract between the Foundry UI host application (1st party) and the child view (3rd party). This is not meant to be directly consumed by any views code, just used internally in Foundry itself.@osdk/views-client.unstable
: This contains the low level client that sets up the listeners and type-safe message passing between the host and child. This will re-export the types from@osdk/views-api.unstable
so that consumers only have to worry about relying on@osdk/views-client*
packages.@osdk/views-react.unstable
: This is a wrapper around@osdk/views-client.unstable
that sets up the listener + React context with parameters in a component@osdk/e2e.sandbox.views.todo-widget
: This is just a test app that let me make sure that all the types worked as intendedI'm calling things
.unstable
for now while we finalize the API. Also note that most of this diff is in the lockfile (which I ranpnpm dedupe
on to avoid duplicate vite/etc versions)Parameters
To start, we support the following parameter types:
YYYY-MM-DD
)Parameters are passed from host UI to child view asynchronously, and the types reflect that (see
AsyncValue
in@osdk/views-api.unstable
).Message passing
Messages are passed between host and child via
postMessage
. The client code that we provide will automatically detect the host origin by looking for a specificmeta
tag that the Foundry view sandbox iframe will inject into the HTML for developers. The messages allowed are defined in@osdk/views-api.unstable
Configuration generation
The intent is that to create a view, the dev just needs a parameters file and a vite plugin (that I have yet to write):
In
entrypoint.parameters.ts
file, they will need to define it like so:In theory, the vite plugin will find the default export from the file that is a sibling to the specified entrypoint file and produce a
view-config.json
file in the output directory that will eventually be uploaded to Foundry to save when the view is published.Usage
The point of having the parameters defined like this is so that we can use some cute type magic to automatically make sure that the messages and parameter values passed back and forth are strongly typed, yet the developer only has to define the parameters once, in the
*.parameters.ts
file. This means that the code can be as simple as this: