Fix virtual module type to properly handle optional properties #13327

smorimoto · 2025-03-29T17:02:29Z

Overview

This PR fixes the TypeScript type definition for the virtual module virtual:react-router/server-build to properly handle optional properties in the ServerBuild interface. The change moves from named exports to a CommonJS-style export = syntax to ensure better TypeScript compatibility, especially with exactOptionalPropertyTypes enabled.

Potential Issues or Risks

Migration impact: The change from named exports to a default export might require modifications in code that specifically imports individual properties from the virtual module
ESM compatibility: As noted in the comments, this approach isn't strictly ESM-compliant, which is a conscious tradeoff made for better typing support

Conclusion

This PR provides a solid solution to type compatibility issues with optional properties in the ServerBuild interface. The approach is well-reasoned and properly tested. The change should improve type-checking accuracy, especially in projects using strict TypeScript configurations.

changeset-bot · 2025-03-29T17:02:33Z

🦋 Changeset detected

Latest commit: 4d600f5

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

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-cla-bot · 2025-03-29T17:02:36Z

Hi @smorimoto,

Welcome, and thank you for contributing to React Router!

Before we consider your pull request, we ask that you sign our Contributor License Agreement (CLA). We require this only once.

You may review the CLA and sign it by adding your name to contributors.yml.

Once the CLA is signed, the CLA Signed label will be added to the pull request.

If you have already signed the CLA and received this response in error, or if you have any questions, please contact us at [email protected].

Thanks!

- The Remix team

remix-cla-bot · 2025-03-29T17:03:20Z

Thank you for signing the Contributor License Agreement. Let's get this merged! 🥳

smorimoto · 2025-03-29T17:06:55Z

This PR will also fix this issue: #13152 (comment)

smorimoto · 2025-03-30T09:50:36Z

@timdorr Thanks a lot for clicking accept for the GitHub Actions workflow, but the previous commit was not a complete fix. Sorry!

@markdalgleish I think you will be interested in this, but maybe this change is not to your liking... (I hope you don't hate it!)

smorimoto · 2025-03-30T09:54:18Z

Off-topic: I believe both contributors and you all could benefit from slightly relaxing the rules around GitHub Actions :)

https://github.com/organizations/remix-run/settings/actions

smorimoto · 2025-03-31T02:38:03Z

The hosted runner lost communication with the server. Anything in your workflow that terminates the runner process, starves it for CPU/Memory, or blocks its network access can cause this error.

🥲

smorimoto · 2025-03-31T12:34:46Z

Ready to go!

packages/react-router-dev/typegen/index.ts

smorimoto · 2025-04-02T21:11:51Z

Can someone review this?

The virtual module `virtual:react-router/server-build` now deliberately implements CommonJS `export =` syntax to correctly support optional properties in the `ServerBuild` type, ensuring proper TypeScript compatibility with `exactOptionalPropertyTypes`. Signed-off-by: Sora Morimoto <[email protected]>

Signed-off-by: Sora Morimoto <[email protected]>

markdalgleish

@smorimoto Thanks for the PR!

After pulling down your PR and trying it out, I noticed that this introduces a different issue which is that the virtual module type now has a default property that doesn't exist in the runtime code.

However, I discovered another solution to the problem that avoids this. By adding | undefined to the types for the optional ServerBuild properties, I'm able to keep your new test passing.

I've pushed an update to your PR with the new implementation but keeping your test case so you still get the credit for helping out here. Hope that's ok! Let me know if this solution looks good from your side and we can get this merged in.

smorimoto · 2025-04-04T13:49:31Z

packages/react-router-dev/typegen/index.ts

@@ -152,7 +152,7 @@ function register(ctx: Context) {

 const virtual = ts`
  declare module "virtual:react-router/server-build" {
-    import { ServerBuild } from "react-router";
+    import type { ServerBuild } from "react-router";    import { ServerBuild } from "react-router";


Suggested change

import type { ServerBuild } from "react-router"; import { ServerBuild } from "react-router";

import type { ServerBuild } from "react-router";

smorimoto · 2025-04-04T13:51:58Z

@markdalgleish I thought exporting never might be the solution, but that does not seem to work...

declare module '...' {
  ...
  export default never; // <- does not work...
  export default undefined // <- same...
}

smorimoto

This change is technically required for compatibility with exactOptionalPropertyTypes. That said, I don't know why the current test does not catch this issue correctly...

smorimoto · 2025-04-04T14:03:54Z

packages/react-router/lib/server-runtime/build.ts

-  basename?: string;
+  // `| undefined` is required to ensure compatibility with TypeScript's
+  // `exactOptionalPropertyTypes` option
+  basename?: string | undefined;


Suggested change

basename?: string | undefined;

basename: string | undefined;

smorimoto · 2025-04-04T14:04:02Z

packages/react-router/lib/server-runtime/build.ts

-  unstable_getCriticalCss?: (args: {
-    pathname: string;
-  }) => OptionalCriticalCss | Promise<OptionalCriticalCss>;
+  unstable_getCriticalCss?:


Suggested change

unstable_getCriticalCss?:

unstable_getCriticalCss:

smorimoto · 2025-04-04T14:07:30Z

@markdalgleish I'm not familiar with Vite's virtual module convention, but on the contrary, is it difficult to publish the default export instead?

remix-cla-bot bot added the CLA Signed label Mar 29, 2025

smorimoto mentioned this pull request Mar 29, 2025

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

Merged

smorimoto force-pushed the fix-virtual-module-type branch from 8648133 to 8b693bf Compare March 30, 2025 09:37

smorimoto changed the title ~~Simplify the type declaration for the virtual module virtual:react-router/server-build~~ Fix virtual module type to properly handle optional properties Mar 30, 2025

smorimoto force-pushed the fix-virtual-module-type branch from 8b693bf to a5ca825 Compare March 30, 2025 09:39

smorimoto commented Mar 31, 2025

View reviewed changes

packages/react-router-dev/typegen/index.ts Outdated Show resolved Hide resolved

smorimoto force-pushed the fix-virtual-module-type branch from a5ca825 to 745a4c0 Compare April 2, 2025 21:10

smorimoto added 2 commits April 3, 2025 06:12

Add @smorimoto to contributors list

017b8cb

Signed-off-by: Sora Morimoto <[email protected]>

smorimoto force-pushed the fix-virtual-module-type branch from 745a4c0 to 017b8cb Compare April 2, 2025 21:12

markdalgleish added 2 commits April 4, 2025 15:51

Update changeset

56149e6

Implement alternate fix via ServerBuild type

4d600f5

markdalgleish reviewed Apr 4, 2025

View reviewed changes

smorimoto commented Apr 4, 2025

View reviewed changes

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Fix virtual module type to properly handle optional properties #13327

Fix virtual module type to properly handle optional properties #13327

smorimoto commented Mar 29, 2025 •

edited

Loading

changeset-bot bot commented Mar 29, 2025 •

edited

Loading

remix-cla-bot bot commented Mar 29, 2025

remix-cla-bot bot commented Mar 29, 2025

smorimoto commented Mar 29, 2025

smorimoto commented Mar 30, 2025

smorimoto commented Mar 30, 2025

smorimoto commented Mar 31, 2025

smorimoto commented Mar 31, 2025

smorimoto commented Apr 2, 2025

markdalgleish left a comment

smorimoto Apr 4, 2025

smorimoto commented Apr 4, 2025 •

edited

Loading

smorimoto left a comment

smorimoto Apr 4, 2025

smorimoto Apr 4, 2025

smorimoto commented Apr 4, 2025

	import type { ServerBuild } from "react-router"; import { ServerBuild } from "react-router";
	import type { ServerBuild } from "react-router";

Fix virtual module type to properly handle optional properties #13327

Are you sure you want to change the base?

Fix virtual module type to properly handle optional properties #13327

Conversation

smorimoto commented Mar 29, 2025 • edited Loading

Overview

Potential Issues or Risks

Conclusion

changeset-bot bot commented Mar 29, 2025 • edited Loading

🦋 Changeset detected

remix-cla-bot bot commented Mar 29, 2025

remix-cla-bot bot commented Mar 29, 2025

smorimoto commented Mar 29, 2025

smorimoto commented Mar 30, 2025

smorimoto commented Mar 30, 2025

smorimoto commented Mar 31, 2025

smorimoto commented Mar 31, 2025

smorimoto commented Apr 2, 2025

markdalgleish left a comment

Choose a reason for hiding this comment

smorimoto Apr 4, 2025

Choose a reason for hiding this comment

smorimoto commented Apr 4, 2025 • edited Loading

smorimoto left a comment

Choose a reason for hiding this comment

smorimoto Apr 4, 2025

Choose a reason for hiding this comment

smorimoto Apr 4, 2025

Choose a reason for hiding this comment

smorimoto commented Apr 4, 2025

smorimoto commented Mar 29, 2025 •

edited

Loading

changeset-bot bot commented Mar 29, 2025 •

edited

Loading

smorimoto commented Apr 4, 2025 •

edited

Loading