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

feat(ssr): support commonjs in ssrTransform #3927

Closed
wants to merge 7 commits into from

Conversation

aleclarson
Copy link
Member

@aleclarson aleclarson commented Jun 23, 2021

Description

When a dependency is (1) cloned within the Vite root and (2) is listed in optimizeDeps.exclude, it will pass through ssrTransform and might be using require and module.exports, so we need to check for that and do some basic replacements. Dependencies in ssr.noExternal are also affected.

Once we know a URL isn't a module (eg: it doesn't use import, export, import.meta or dynamic import(...)), we can do the following replacements:

  • replace require calls with __vite_ssr_import__
  • replace module.exports and exports references with __vite_ssr_exports__
  • replace module.exports assignments with __vite_ssr_exports__.default = xxx

This PR also moves __esModule declaration into ssrTransform so CJS modules can be made compatible with ESM import by automatically assigning exports.default = exports within ssrModuleLoader if it's undefined. This allows import foo from 'cjs' to work as expected.

Additional context

Fixes #2579


What is the purpose of this pull request?

  • Bug fix
  • New Feature
  • Documentation update
  • Other

Before submitting the PR, please make sure you do the following

  • Read the Contributing Guidelines.
  • Read the Pull Request Guidelines and follow the Commit Convention.
  • Check that there isn't already a PR that solves the problem the same way to avoid creating a duplicate.
  • Provide a description in this PR that addresses what the PR is solving, or reference the issue that it solves (e.g. fixes #123).
  • Ideally, include relevant tests that fail without this PR but pass with it.

When a dependency is (1) cloned within the Vite root and (2) is listed in `optimizeDeps.exclude`, it will pass through `ssrTransform` and might be using `require` and `module.exports`, so we need to check for that and do some basic replacements.

Drawbacks include:
- Comments and strings containing `module.exports` or `exports` are not accounted for, so they will be rewritten.

Fixes vitejs#2579
if (!ssrModule.__esModule) {
Object.defineProperty(ssrModule, '__esModule', { value: true })
if (!Object.getOwnPropertyDescriptor(ssrModule, 'default')) {
Object.defineProperty(ssrModule, 'default', { value: ssrModule })
Copy link
Member Author

Choose a reason for hiding this comment

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

This makes a CommonJS module compatible with ESM default import.

ES modules are not affected, since ssrTransform now injects a Object.defineProperty call when ESM syntax is detected.

@aleclarson aleclarson added the p5-urgent Fix build-breaking bugs affecting most users, should be released ASAP (priority) label Jun 24, 2021
@antfu antfu added p4-important Violate documented behavior or significantly improves performance (priority) p3-minor-bug An edge case that only affects very specific usage (priority) and removed p5-urgent Fix build-breaking bugs affecting most users, should be released ASAP (priority) labels Jun 25, 2021
@aleclarson aleclarson removed the p4-important Violate documented behavior or significantly improves performance (priority) label Jun 25, 2021
@aleclarson
Copy link
Member Author

Turns out that optimizeDeps.exclude does not support CJS modules, which is by design.

Closing in favor of a docs PR that mentions this caveat.

@aleclarson aleclarson closed this Jun 25, 2021
@aleclarson aleclarson deleted the feat/ssr-commonjs branch February 25, 2022 20:03
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
p3-minor-bug An edge case that only affects very specific usage (priority)
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Bundling SSR Modules using CommonJS exports alias results in ReferenceError: exports is not defined
2 participants