-
Notifications
You must be signed in to change notification settings - Fork 159
Tech design: Runtime context refactor #8590
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
base: main
Are you sure you want to change the base?
Conversation
|
|
||
| Each subtree uses its own transport and instanceId. Components don't need to know which runtime they're in. | ||
|
|
||
| ## Code Generator |
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.
Is there a library for this already?
| }); | ||
|
|
||
| const client = createRuntimeServiceClient(transport); | ||
| const explore = await client.getExplore({ |
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.
This doesn't populate the tanstack query cache right? We might need to restructure our generated code to support something direct fetching with cache population.
Or avoid loader functions altogether.
|
|
||
| ## Open Questions | ||
|
|
||
| 1. **Query key namespacing:** Should query keys include a version or hash to handle proto schema changes? |
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.
What does this give us? Wouldnt the backend APIs not exist for the previous endpoints anymore?
| // Switch to viewing as another user | ||
| async function impersonateUser(userId: string) { | ||
| const newJwt = await adminService.getImpersonationToken(userId); | ||
| // Update RuntimeProvider props → new transport created → queries refetch |
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.
Would this mean this function will have to sit above the <RuntimeProvider> wrap? Would be good to flesh this section out to make sure there are no issues.
|
One thing that this touches on, but does not fully incorporate is the dependency between runtime data and a project. I think bringing the terms host/runtime/instanceId to the forefront confuses what is actually happening, which is that a user is querying something in a project. To that end, I don't think we should have a I also think the Here's a very rough working implementation of what I'm imagining: #8603 |
Seeking feedback. Not to merge to main.
Brian's PRs #8559 and #8572 tackle the runtime context issues from different angles. I spent some time digging into the problem and wrote up this document to clarify the options and propose a direction.
The short version:
Looking for feedback on: