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

SPA Mode #8338

Merged
merged 31 commits into from Jan 8, 2024
Merged

SPA Mode #8338

merged 31 commits into from Jan 8, 2024

Conversation

brophdawg11
Copy link
Contributor

@brophdawg11 brophdawg11 commented Dec 20, 2023

Todo:

RFC: #7638
Closes: #7639

Copy link

changeset-bot bot commented Dec 20, 2023

🦋 Changeset detected

Latest commit: 525e424

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

This PR includes changesets to release 16 packages
Name Type
@remix-run/dev Minor
@remix-run/react Minor
@remix-run/server-runtime Minor
@remix-run/testing Minor
@remix-run/cloudflare Minor
@remix-run/deno Minor
@remix-run/node Minor
@remix-run/cloudflare-pages Minor
@remix-run/cloudflare-workers Minor
@remix-run/architect Minor
@remix-run/express Minor
@remix-run/serve Minor
create-remix Minor
remix Minor
@remix-run/css-bundle Minor
@remix-run/eslint-config Minor

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

"@remix-run/node"
);
let handler = createNodeRequestHandler(build, mode);
let response = await handler(new Request("http://localhost/"));
Copy link
Contributor

Choose a reason for hiding this comment

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

Please consider the suggestion documented at #7638 (reply in thread) to allow either an index.html or index.js to be generated. It's the smallest change which allows the generation of a Remix App library file that I could think of but maybe the team has better ideas.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Still pondering this, but we're going to get this out as an unstable_ssr flag without this functionality to start. Hopefully that'll let some folks play around with it and we can gather some feedback and continue to evaluate this use case.

if (
initialPathname !== hydratedPathname &&
!window.__remixContext.isSpaMode
) {
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Don't perform this check in SPA mode since we want to be able to hydrate any URL via route.lazy

// hydration
hydrationData.loaderData[routeId] = null;
let hydrationData = undefined;
if (!window.__remixContext.isSpaMode) {
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 just a whitespace change - we don't have to prep any hydrationData in SPA mode

) {
return (
routeModule.clientLoader != null &&
(routeModule.clientLoader.hydrate === true || route.hasLoader !== true)
(isSpaMode && route.id !== "root") ||
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Never hydrate below the root in SPA mode

@marbemac

This comment was marked as resolved.

@pcattori

This comment was marked as resolved.

@brophdawg11 brophdawg11 changed the base branch from dev to feature/spa-mode January 5, 2024 16:23
@brophdawg11 brophdawg11 marked this pull request as ready for review January 5, 2024 16:23
@Abbbdab

This comment was marked as off-topic.

@aarjithn

This comment was marked as off-topic.

@brophdawg11

This comment was marked as off-topic.

Copy link
Member

@jacob-ebey jacob-ebey left a comment

Choose a reason for hiding this comment

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

useId test please :)

@brophdawg11 brophdawg11 merged commit e16704f into feature/spa-mode Jan 8, 2024
9 checks passed
@brophdawg11 brophdawg11 deleted the brophdawg11/spa-mode branch January 8, 2024 21:16
@brophdawg11 brophdawg11 mentioned this pull request Jan 8, 2024
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.

None yet

9 participants