Skip to content

fix: interop of cjs with_esModule is consistent with rollup#16437

Closed
XiSenao wants to merge 4 commits intovitejs:mainfrom
XiSenao:fix/default-interop-consistency
Closed

fix: interop of cjs with_esModule is consistent with rollup#16437
XiSenao wants to merge 4 commits intovitejs:mainfrom
XiSenao:fix/default-interop-consistency

Conversation

@XiSenao
Copy link
Collaborator

@XiSenao XiSenao commented Apr 16, 2024 •
edited
Loading

Description

CJS packages with __esModule require consistent interop behavior with rollup during development phase.

@bolt-new-by-stackblitz
Copy link

bolt-new-by-stackblitz bot commented Apr 16, 2024

Review PR in StackBlitz Codeflow Run & review this pull request in StackBlitz Codeflow.

@XiSenao XiSenao added p3-minor-bug An edge case that only affects very specific usage (priority) inconsistency Inconsistency between dev & build p2-edge-case Bug, but has workaround or limited in scope (priority) and removed p3-minor-bug An edge case that only affects very specific usage (priority) labels Apr 17, 2024
@sapphi-red sapphi-red added has workaround feat: deps optimizer Esbuild Dependencies Optimization labels Apr 17, 2024
sapphi-red
sapphi-red previously approved these changes Apr 17, 2024
View reviewed changes
Copy link
Member

@sapphi-red sapphi-red left a comment •
edited
Loading

Choose a reason for hiding this comment

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

The change looks good to me. Would you add a test in playground/optimize-deps?

@sapphi-red

This comment was marked as outdated.

Sign in to view
@vite-ecosystem-ci

This comment was marked as outdated.

Sign in to view
@XiSenao XiSenao force-pushed the fix/default-interop-consistency branch from 02d6402 to aeaf129 Compare April 18, 2024 06:23
@XiSenao
Copy link
Collaborator Author

XiSenao commented Apr 18, 2024

/ecosystem-ci run

@XiSenao XiSenao changed the title fix: default imported interop is synchronized with rollup. fix: interop of cjs with_esModule is consistent with rollup Apr 18, 2024
@vite-ecosystem-ci
Copy link

vite-ecosystem-ci bot commented Apr 18, 2024

📝 Ran ecosystem CI on aeaf129: Open

suite result latest scheduled
nuxt ✅ success ❌ failure

✅ analogjs, astro, histoire, ladle, laravel, marko, previewjs, quasar, qwik, rakkas, remix, sveltekit, unocss, vike, vite-plugin-pwa, vite-plugin-react, vite-plugin-react-pages, vite-plugin-react-swc, vite-plugin-svelte, vite-plugin-vue, vite-setup-catalogue, vitepress, vitest

@XiSenao XiSenao requested review from bluwy and sapphi-red April 18, 2024 06:38
@bluwy
Copy link
Member

bluwy commented Apr 18, 2024 •
edited
Loading

I wonder if the Rollup behaviour is correct though. __esModule means that if you default import a module, you'd access {}.default. If we have to check whether .default exist, then why would the module mark __esModule in the first place, and I think the reason is that it doesn't want to allow modules to default import it instead, only named imports. And so the current behaviour is correct, and we should improve the error message instead?

For consistency with Rollup, I'm ok if we want to go ahead with this first, but I think we should re-evaluate if this is the correct behaviour.

@patak-dev
Copy link
Member

patak-dev commented Apr 18, 2024

About Nuxt, it is also failing in main right now.

@XiSenao
Copy link
Collaborator Author

XiSenao commented Apr 18, 2024 •
edited
Loading

I think the behavior of rollup may be considering compatibility with Node (esbuild and webpack also do similar things). If we follow Babel's explanation, for the current scenario (where __esModule is true and the default export is not provided), I think it would be good to provide an error similar to "SyntaxError: The requested module 'xxx' does not provide an export named 'default'". I may lean more towards the latter, but it seems that there is no standardization for __esModule in the community and namespace import may not be friendly, which still appears chaotic to me 😂.

@sapphi-red
Copy link
Member

sapphi-red commented Apr 18, 2024

Given that we cannot support both Node's way and Babel's way, I think it's a good middle-ground. It seems it was discussed at rollup/plugins#537.

bluwy
bluwy reviewed May 2, 2024
View reviewed changes
packages/vite/src/node/plugins/importAnalysis.ts
}
// prettier-ignore
const interopHelper = (m: any) => m?.__esModule ? m : { ...(typeof m === 'object' && !Array.isArray(m) || typeof m === 'function' ? m : {}), default: m }
const interopHelper = (m: any) => m?.__esModule && Object.prototype.hasOwnProperty.call(m, 'default') ? m : { ...(typeof m === 'object' && !Array.isArray(m) || typeof m === 'function' ? m : {}), default: m }
Copy link
Member

@bluwy bluwy May 2, 2024

Choose a reason for hiding this comment

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

Let's go ahead with this then to keep dev/build consistent for now. I think we should also add a TODO comment here to revisit this later in Vite 6.

@sapphi-red
Copy link
Member

sapphi-red commented Jan 29, 2026

Closing as this was fixed in Vite 8 beta.

@sapphi-red sapphi-red closed this Jan 29, 2026
Copilot AI mentioned this pull request Jan 29, 2026
Merged
Copilot AI added a commit that referenced this pull request Jan 29, 2026
@sapphi-red
Add test files and dependencies from PR #16437
f2eac6e 
Co-authored-by: sapphi-red <49056869+sapphi-red@users.noreply.github.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@bluwy bluwy bluwy left review comments

@sapphi-red sapphi-red Awaiting requested review from sapphi-red

Assignees

No one assigned

Labels

feat: deps optimizer Esbuild Dependencies Optimization has workaround inconsistency Inconsistency between dev & build p2-edge-case Bug, but has workaround or limited in scope (priority)

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

Vite dev server is unable to import @babel/standalone

4 participants

@XiSenao @sapphi-red @bluwy @patak-dev