-
Notifications
You must be signed in to change notification settings - Fork 991
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
feat: Send RSC Flight Payload to Studio #10213
Conversation
@Tobbe Probably good to get this reviewed and any refactor ideas since where the send to Studio is added is at the end of a rather long function; so, open to suggestions. |
@Tobbe One question I had is if I should check the toml config for the Studio options -- and use that as a conditional to send or not to send to Studio? Or should we have an additional setting -- and if so, which? |
@dthyresson Can you please fix the linting errors and then I'll have another look |
I just noticed those -- will do. |
Co-authored-by: Tobbe Lundberg <tobbe@tlundberg.com>
Co-authored-by: Tobbe Lundberg <tobbe@tlundberg.com>
I had to fix the linter comma issues with On saving the file, my prettier config seems to remove them. Perhaps my prettier is out of date? |
Also, I imagine I should not send to Studio during CI or test -- so, will want a way to disable this as well in those cases. Although: |
const startedAt = Date.now() | ||
const start = performance.now() | ||
const pipeable = await renderRsc({ rscId, props, rsfId, args }) | ||
const endedAt = Date.now() | ||
const end = performance.now() | ||
const duration = end - start |
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.
Just out of interest was it easier to use these different .now
functions rather than having startAt
come from the performance.now
? Is performance.now
difficult to turn back into a JS Date object? Just interested, don't mean to be nitpicker or anything
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 wanted to track start and end date times (proper timestamps) so startedAt, endedAt.
The performance start and end are some msec since Node starts something, so just used for the duration.
In the metadata I send to Studio, I send the startedAt, endedAt and duration:
performance: {
startedAt,
endedAt,
duration,
},
but not those other time values -- because, yes, near impossible to transform back into date times.
@Tobbe I refactored to get the Studio port from the config and only send to Studio when needed. Note that I have it sending in production now as apps run in production -- so need this for testing and demos. I also render RSC a second time just for Studio so I can wrap that rendering in performance only for Studio and never in production. This may add a tiny perf fit when in Dev with Studio, but seems reasonable to not add those perf checks in production? |
@Tobbe I made the logic change to const shouldSendToStudio = () => {
// TODO (RSC): This should be just isDevelopment()
// but since RSC apps currently run in production mode
// we need to check for 'production' (aka not 'development')
// for now when sending to Studio
return isStudioEnabled() && !isDevelopment()
} |
At the time of #10213 the changeset CLI utility generated the name as a random three-word combo. This was later updated to be the PR number instead. This PR renames that one older entry so the whole folder has the same naming convention. Getting a review on this to ensure this won't break some tooling/state that I'm not as familiar with.
At the time of #10213 the changeset CLI utility generated the name as a random three-word combo. This was later updated to be the PR number instead. This PR renames that one older entry so the whole folder has the same naming convention. Getting a review on this to ensure this won't break some tooling/state that I'm not as familiar with.
This PR sends the rendered RSC payload (aka "flight") to Studio to be ingested, persisted, and visualized. See: redwoodjs/studio#52
Am encoding the payload as base64 text as a convenience so it gets persisted as encoded text an not some unwieldily JSON with escape characters. Studio will decoded and handle the flight payload accordingly.
This is a proof of concept and still many todos:
Studio
and single preview for the moment ... still UI todo