Skip to content

Conversation

jiji-hoon96
Copy link
Contributor

Description

Fixes #20840

This PR moves the isBuiltin check from the server side to the module runner side to avoid unnecessary server round-trips when importing builtin modules (e.g., node:fs, node:path).

Module runner checks builtin modules client-side first. The server-side check remains as a fallback for custom runner implementations.

Changes

  • Added createIsBuiltin utility in module-runner/utils.ts
  • Added builtins option to ModuleRunnerOptions interface
  • Module runner performs client-side builtin check before calling server
  • createServerModuleRunner automatically passes builtins config to runner

@sapphi-red sapphi-red added p2-nice-to-have Not breaking anything but nice to have (priority) feat: ssr performance Performance related enhancement labels Oct 15, 2025
@sapphi-red sapphi-red changed the title feat: add client-side builtin module check in module runner perf: add client-side builtin module check in module runner Oct 15, 2025
@jiji-hoon96
Copy link
Contributor Author

@sapphi-red Thank you for your feedback.

The requested modifications have been implemented.
Instead of passing options, we've changed it to use transport invoke to fetch built-in functions from the server.

The builtins option is now optional and will be automatically fetched from the server upon first use.

We have also verified that the tests related to the modified runner.ts file are functioning correctly.
Please let us know if any further code changes are needed. Thank you.

@sapphi-red
Copy link
Member

sapphi-red commented Oct 19, 2025

Would you add a test to packages/vite/src/node/ssr/runtime/__tests__/server-worker-runner.invoke.spec.ts that resolves an id included in isBuiltin?

@jiji-hoon96
Copy link
Contributor Author

Sure! I'll add a test to server-worker-runner.invoke.spec.ts

@sapphi-red sapphi-red changed the title perf: add client-side builtin module check in module runner perf(module-runner): add client-side builtin module check Oct 20, 2025
Copy link
Member

@sapphi-red sapphi-red left a comment

Choose a reason for hiding this comment

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

Thanks! I've pushed minor changes while resolving the conflict.

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

Labels

feat: ssr p2-nice-to-have Not breaking anything but nice to have (priority) performance Performance related enhancement

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Do the isBuiltin check on module runner side

2 participants