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

Support for client side context #12515

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

Conversation

brophdawg11
Copy link
Contributor

No description provided.

Copy link

changeset-bot bot commented Dec 10, 2024

🦋 Changeset detected

Latest commit: 2a92d5e

The changes in this PR will be included in the next version bump.

This PR includes changesets to release 11 packages
Name Type
react-router Major
@react-router/architect Major
@react-router/cloudflare Major
@react-router/dev Major
react-router-dom Major
@react-router/express Major
@react-router/node Major
@react-router/serve Major
@react-router/fs-routes Major
@react-router/remix-routes-option-adapter Major
create-react-router Major

Not sure what this means? Click here to learn what changesets are.

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

@@ -85,6 +85,7 @@ export type {
AwaitProps,
IndexRouteProps,
LayoutRouteProps,
MemoryRouterOpts,
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Export this and DOMRouterOpts so they get picked up by typedoc

@@ -130,23 +131,62 @@ export function mapRouteProperties(route: RouteObject) {
return updates;
}

export interface MemoryRouterOpts {
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Extracted to an interface so it gets it's own docs page and to align with DOMRouterOpts

/**
* Router context singleton that will be passed to loader/action functions.
*/
context?: DefaultRouterContext;
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 the only new API:

let router = createMemoryRouter(routes, {
  context: { user: getUser() }
});

@@ -49,10 +45,6 @@ interface StubNonIndexRouteObject

type StubRouteObject = StubIndexRouteObject | StubNonIndexRouteObject;

interface AppLoadContext {
[key: string]: unknown;
}
Copy link
Contributor Author

Choose a reason for hiding this comment

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

No longer need this duped with the repo merge and we can import the actual one

@@ -153,21 +145,15 @@ function processRoutes(
);
}

// Patch in the Remix context to loaders/actions
Copy link
Contributor Author

Choose a reason for hiding this comment

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

No longer needed 🫡

{ request }: LoaderFunctionArgs,
singleFetch?: unknown
) =>
dataRoute.loader = (_: LoaderFunctionArgs, singleFetch?: unknown) =>
Copy link
Contributor Author

Choose a reason for hiding this comment

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

request was unused here

@@ -812,6 +814,7 @@ export function createRouter(init: RouterInit): Router {
);
let inFlightDataRoutes: AgnosticDataRouteObject[] | undefined;
let basename = init.basename || "/";
let routerContext = typeof init.context !== "undefined" ? init.context : {};
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Default context to {} just like in Remix/SSR

@@ -3676,7 +3679,7 @@ export function createStaticHandler(
? actionMatch
: findNearestBoundary(matches, actionMatch.route.id);

let context = await loadRouteData(
let handlerContext = await loadRouteData(
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Rename for clarity:

  • handlerContext-> a StaticHandlerContext returned from staticHandler.query()
  • requestContext -> The AppLoadContext used passed to staticHandler.query by Remix SSR today
  • routerContext -> This new DefaultRouterContext used for SPAs since it's not tired to a request like the SSR use case

In shared code, requestContext becomes routerContext

@@ -118,19 +125,19 @@ export type Submission =
interface DataFunctionArgs<Context> {
request: Request;
params: Params;
context?: Context;
context: Context;
Copy link
Contributor Author

Choose a reason for hiding this comment

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

No longer optional - still maintains a default type of any though

request,
matchesToLoad,
matches,
fetcherKey,
manifest,
mapRouteProperties
mapRouteProperties,
routerContext
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 the core functional addition - the shared dataStrategy stuff already supports context for SSR, we just didn't used to pass anything for client side usages - now we do

@@ -2722,13 +2725,13 @@ export function createRouter(init: RouterInit): Router {
results = await callDataStrategyImpl(
dataStrategyImpl,
type,
state,
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 param was unused

request: Request,
matchesToLoad: AgnosticDataRouteMatch[],
matches: AgnosticDataRouteMatch[],
fetcherKey: string | null,
manifest: RouteManifest,
mapRouteProperties: MapRoutePropertiesFunction,
requestContext?: unknown
routerContext: unknown
Copy link
Contributor Author

Choose a reason for hiding this comment

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

No longer optional since it's used in both SSR and SPA use cases

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