Skip to content

Comments

fix(ssr): combine empty source mappings#19226

Merged
patak-dev merged 1 commit intovitejs:mainfrom
hi-ogawa:fix-ssr-transform-collpase-empty
Jan 23, 2025
Merged

fix(ssr): combine empty source mappings#19226
patak-dev merged 1 commit intovitejs:mainfrom
hi-ogawa:fix-ssr-transform-collpase-empty

Conversation

@hi-ogawa
Copy link
Contributor

@hi-ogawa hi-ogawa commented Jan 19, 2025

Description

Current SSR transform skips collapsing map: { mappings: '' } and uses magic-string generated map as is. However this is incorrect since empty mappings should get collapsed to empty mappings. Additionally, magic-string source map generation is unnecessary, so it can be skipped in this case.

This change fixes a perf hit when plugin returns a large code from load or transform with explicit empty mappping map: { mappings: '' } as par rollup convention https://rollupjs.org/plugin-development/#source-code-transformations.

@hi-ogawa hi-ogawa marked this pull request as ready for review January 19, 2025 02:11
Copy link
Member

@bluwy bluwy left a comment

Choose a reason for hiding this comment

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

Nice find!

@patak-dev patak-dev added this to the 6.1 milestone Jan 20, 2025
@patak-dev patak-dev merged commit ba03da2 into vitejs:main Jan 23, 2025
18 checks passed
moonlitusun pushed a commit to moonlitusun/vite that referenced this pull request May 25, 2025
blake-newman added a commit to blake-newman/vitest that referenced this pull request Jun 2, 2025
vitejs/vite#19226 introduced a change where by empty source
mappings are not combined. As such when vitest has an internal error caused
such as using jsdom feature that is not implemented which logs an error
it tries to map the source map when the mapping is empty which results
in an error such as:

```
Cannot read properties of undefined (reading 'map')
 ❯ VirtualConsole.<anonymous> node_modules/.pnpm/[email protected]/node_modules/jsdom/lib/jsdom/virtual-console.js:29:53
 ❯ module.exports node_modules/.pnpm/[email protected]/node_modules/jsdom/lib/jsdom/browser/not-implemented.js:12:26
 ❯ HTMLAudioElementImpl.load node_modules/.pnpm/[email protected]/node_modules/jsdom/lib/jsdom/living/nodes/HTMLMediaElement-impl.js:112:5
 ❯ HTMLAudioElement.load node_modules/.pnpm/[email protected]/node_modules/jsdom/lib/jsdom/living/generated/HTMLMediaElement.js:109:34
```

When the error originates from a file without source mapping such as `x.vue`
it errors instead of logging original error.
blake-newman added a commit to blake-newman/vitest that referenced this pull request Jun 2, 2025
vitejs/vite#19226 introduced a change where by empty source
mappings are not combined. As such when vitest has an internal error caused
such as using jsdom feature that is not implemented which logs an error
it tries to map the source map when the mapping is empty which results
in an error such as:

```
Cannot read properties of undefined (reading 'map')
 ❯ VirtualConsole.<anonymous> node_modules/.pnpm/[email protected]/node_modules/jsdom/lib/jsdom/virtual-console.js:29:53
 ❯ module.exports node_modules/.pnpm/[email protected]/node_modules/jsdom/lib/jsdom/browser/not-implemented.js:12:26
 ❯ HTMLAudioElementImpl.load node_modules/.pnpm/[email protected]/node_modules/jsdom/lib/jsdom/living/nodes/HTMLMediaElement-impl.js:112:5
 ❯ HTMLAudioElement.load node_modules/.pnpm/[email protected]/node_modules/jsdom/lib/jsdom/living/generated/HTMLMediaElement.js:109:34
```

When the error originates from a file without source mapping such as `x.vue`
it errors instead of logging original error.
blake-newman added a commit to blake-newman/vitest that referenced this pull request Jun 2, 2025
vitejs/vite#19226 introduced a change where by empty source
mappings are not combined. As such when vitest has an internal error caused
such as using jsdom feature that is not implemented which logs an error
it tries to map the source map when the mapping is empty which results
in an error such as:

```
Cannot read properties of undefined (reading 'map')
 ❯ VirtualConsole.<anonymous> node_modules/.pnpm/[email protected]/node_modules/jsdom/lib/jsdom/virtual-console.js:29:53
 ❯ module.exports node_modules/.pnpm/[email protected]/node_modules/jsdom/lib/jsdom/browser/not-implemented.js:12:26
 ❯ HTMLAudioElementImpl.load node_modules/.pnpm/[email protected]/node_modules/jsdom/lib/jsdom/living/nodes/HTMLMediaElement-impl.js:112:5
 ❯ HTMLAudioElement.load node_modules/.pnpm/[email protected]/node_modules/jsdom/lib/jsdom/living/generated/HTMLMediaElement.js:109:34
```

When the error originates from a file without source mapping such as `x.vue`
it errors instead of logging original error.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Ability to disable sourcemap generation in dev for large files, i.e Astro 5's data layer

3 participants