Skip to content
New issue

Have a question about this project? # for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “#”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? # to your account

fix(ssr): preserve require for external node #11057

Merged

Conversation

sapphi-red
Copy link
Member

@sapphi-red sapphi-red commented Nov 24, 2022

Description

Depends on #10406. The change is 3789d8f.

This fixes what #10450 tried to fix.

When

  • an app depends on A
  • A depends on B
  • A and B are both written in CJS
  • B is externalized
  • A is no-externalized

require('B') written in A is converted to import 'B'. But converting require into import is not always safe. For example, if B uses exports field and uses different files.
This PR fixes this by preserving require as-is for SSR with node target.

Additional context


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.

@sapphi-red sapphi-red added p3-minor-bug An edge case that only affects very specific usage (priority) feat: ssr feat: deps optimizer Esbuild Dependencies Optimization labels Nov 24, 2022
@sapphi-red sapphi-red force-pushed the fix/preserve-require-for-external-node branch from 3789d8f to f138d52 Compare November 24, 2022 15:01
@sapphi-red sapphi-red marked this pull request as draft November 29, 2022 17:19
@sapphi-red sapphi-red marked this pull request as ready for review November 30, 2022 03:36
@patak-dev patak-dev merged commit 1ec0176 into vitejs:main Nov 30, 2022
@sapphi-red sapphi-red deleted the fix/preserve-require-for-external-node branch November 30, 2022 06:42
futurGH pushed a commit to futurGH/vite that referenced this pull request Feb 26, 2023
# for free to join this conversation on GitHub. Already have an account? # to comment
Labels
feat: deps optimizer Esbuild Dependencies Optimization feat: ssr 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.

3 participants