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

Generate types for virtual:react-router/server-build #13152

Merged
merged 1 commit into from
Mar 4, 2025

Conversation

markdalgleish
Copy link
Member

@markdalgleish markdalgleish commented Mar 4, 2025

It's invalid to declare a module called virtual:react-router/server-build within an npm package, but it's valid within app code. In order to provide types for this virtual module, this PR writes a new file as part of the typegen flow that declares this module.

Shout out to Astro for this. They use a similar approach for their astro:-prefixed virtual modules.

Copy link

changeset-bot bot commented Mar 4, 2025

🦋 Changeset detected

Latest commit: 541fee1

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/dev Major
@react-router/fs-routes Major
@react-router/remix-routes-option-adapter Major
create-react-router Major
react-router Major
react-router-dom Major
@react-router/architect Major
@react-router/cloudflare Major
@react-router/express Major
@react-router/node Major
@react-router/serve 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

@@ -756,7 +756,7 @@ export const reactRouterVitePlugin: ReactRouterVitePlugin = () => {
ctx.reactRouterConfig.future.unstable_viteEnvironmentApi &&
viteCommand === "serve"
? `
export const getCriticalCss = ({ pathname }) => {
export const unstable_getCriticalCss = ({ pathname }) => {
Copy link
Member Author

@markdalgleish markdalgleish Mar 4, 2025

Choose a reason for hiding this comment

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

Seeing this API in a new light as part of this PR made me realise this should be marked as unstable. It was only ever defined when setting the future.unstable_viteEnvironmentApi flag.

@@ -14,7 +14,6 @@ declare module "react-router" {
}

const requestHandler = createRequestHandler(
// @ts-expect-error
Copy link
Member Author

Choose a reason for hiding this comment

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

Removing this line is what this PR is all about 😄

@markdalgleish markdalgleish merged commit f77ca63 into dev Mar 4, 2025
8 checks passed
@markdalgleish markdalgleish deleted the markdalgleish/virtual-server-build-types branch March 4, 2025 23:50
wilcoxmd added a commit to wilcoxmd/react-router that referenced this pull request Mar 5, 2025
…d-route-typegen

* upstream/dev: (65 commits)
  Generate types for `virtual:react-router/server-build` (remix-run#13152)
  Add support for client context and middleware (unstable) (remix-run#12941)
  Add playground for `vite-plugin-cloudflare` (remix-run#13151)
  do not typegen params for layout routes with a corresponding index (remix-run#13131) (remix-run#13140)
  Fix types for `loaderData` and `actionData` that contain `Record`s (remix-run#13139)
  chore: format
  chore(dev): remove unused dependencies (remix-run#13134)
  Remove unused Vite file system watcher (remix-run#13133)
  Remove stale changesets cherry-picked into release-next for 7.2.0
  Fix custom SSR build input with `serverBundles` (remix-run#13107)
  Skip resource route flow in dev mode when SPA  mode is enabled (remix-run#13113)
  chore: format
  Add integration test for `vite-plugin-cloudflare` (remix-run#13099)
  Fix custom client `build.rollupOptions.output.entryFileNames` (remix-run#13098)
  Detect lazy route discovery manifest version mismatches and trigger reloads (remix-run#13061)
  Fix critical CSS with custom `Vite.DevEnvironment` (remix-run#13066)
  Fix usage of `prerender` option with `serverBundles` (remix-run#13082)
  Fix support for custom `build.assetsDir` (remix-run#13077)
  Add changeset for remix-run#13064
  Only import the root route when SSRing SPA mode's index.html (remix-run#13023)
  ...
Copy link
Contributor

github-actions bot commented Mar 6, 2025

🤖 Hello there,

We just published version 7.3.0-pre.1 which includes this pull request. If you'd like to take it for a test run please try it out and let us know what you think!

Thanks!

Copy link
Contributor

🤖 Hello there,

We just published version 7.4.0 which includes this pull request. If you'd like to take it for a test run please try it out and let us know what you think!

Thanks!

@smorimoto
Copy link

This change is fantastic, but will cause an error if exactOptionalPropertyTypes is enabled. If the generated code goes like this, it will work fine, but I'm not sure if this is OK...

declare module "virtual:react-router/server-build" {
  import { ServerBuild } from "react-router";
  export const assets: ServerBuild["assets"];
  export const assetsBuildDirectory: ServerBuild["assetsBuildDirectory"];
- export const basename: ServerBuild["basename"];
+ export const basename: NonNullable<ServerBuild["basename"]>;
  export const entry: ServerBuild["entry"];
  export const future: ServerBuild["future"];
  export const isSpaMode: ServerBuild["isSpaMode"];
  export const prerender: ServerBuild["prerender"];
  export const publicPath: ServerBuild["publicPath"];
  export const routes: ServerBuild["routes"];
  export const ssr: ServerBuild["ssr"];
- export const unstable_getCriticalCss: ServerBuild["unstable_getCriticalCss"];
+ export const unstable_getCriticalCss: NonNullable<ServerBuild["unstable_getCriticalCss"]>;
}

@markdalgleish
Copy link
Member Author

@smorimoto I've tried running npx create-react-router@latest to generate a fresh project and then enabled exactOptionalPropertyTypes but I've been unable to reproduce it. Are you able to share a minimal repro?

@smorimoto
Copy link

#13267 fixes the issue!

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

3 participants