Skip to content
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

Refactor dataStrategy for easier RSC abstraction #13344

Open
wants to merge 6 commits into
base: dev
Choose a base branch
from

Conversation

brophdawg11
Copy link
Contributor

No description provided.

Copy link

changeset-bot bot commented Apr 2, 2025

⚠️ No Changeset found

Latest commit: d4c0d2c

Merging this PR will not cause a version bump for any packages. If these changes should not result in a new version, you're good to go. If these changes should result in a version bump, you need to add a changeset.

This PR includes no changesets

When changesets are added to this PR, you'll see the packages that this PR includes changesets for and the associated semver types

Click here to learn what changesets are, and how to add one.

Click here if you're a maintainer who wants to add a changeset to this PR

@brophdawg11 brophdawg11 force-pushed the brophdawg11/single-fetch-refactor branch from 122598c to 12857fd Compare April 2, 2025 19:51
@brophdawg11 brophdawg11 force-pushed the brophdawg11/single-fetch-refactor branch from 8f33065 to 573f1a1 Compare April 3, 2025 14:57
@brophdawg11 brophdawg11 force-pushed the brophdawg11/single-fetch-refactor branch from a5c98a5 to 5047f3f Compare April 3, 2025 15:02
};
export type SingleFetchResults =
| { [key: string]: SingleFetchResult }
| { [SingleFetchRedirectSymbol]: SingleFetchRedirectResult };
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is now correctly represented as a union - we only ever return data or a redirect from the server - not both

Comment on lines +34 to +37
// Shared/serializable type used by both turbo-stream and RSC implementations
type DecodedSingleFetchResults =
| { routes: { [key: string]: SingleFetchResult } }
| { redirect: SingleFetchRedirectResult };
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

fetchAndDecode will now return this shape which can be the common payload for turbo-stream and RSC. unwrapSingleFetchResult now operates on this so that implementation can be shared.

// treat those as "no data" instead of throwing an exception.
// 304 is not included here because the browser should fill those responses
// with the cached body content.
export const NO_BODY_STATUS_CODES = new Set([100, 101, 204, 205]);
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Lifted from below so we can export for consumption in server-runtime code

Comment on lines -319 to +342
// Track which routes need a server load - in case we need to tack on a
// `_routes` param
// Track which routes need a server load for use in a `_routes` param
let routesParams = new Set<string>();

// We only add `_routes` when one or more routes opts out of a load via
// `shouldRevalidate` or `clientLoader`
// Only add `_routes` when at least 1 route opts out via `shouldRevalidate`/`clientLoader`
let foundOptOutRoute = false;

// Deferreds for each route so we can be sure they've all loaded via
// `match.resolve()`, and a singular promise that can tell us all routes
// have been resolved
// Deferreds per-route so we can be sure they've all loaded via `match.resolve()`
let routeDfds = matches.map(() => createDeferred<void>());
let routesLoadedPromise = Promise.all(routeDfds.map((d) => d.promise));

// Deferred that we'll use for the call to the server that each match can
// await and parse out it's specific result
let singleFetchDfd = createDeferred<SingleFetchResults>();
// Deferred we'll use for the singleular call to the server
let singleFetchDfd = createDeferred<DecodedSingleFetchResults>();
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I went back to the single deferred approach since we can now avoid dual fetchAndDecode/unwrap DI needs by decoding to a shared shape in fetchAndDecode - so mostly just comment updates leftover

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

1 participant