Skip to content

fix(ssr): combine empty source mappings#19226

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

fix(ssr): combine empty source mappings#19226
patak-cat 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-cat patak-cat added this to the 6.1 milestone Jan 20, 2025
@patak-cat patak-cat merged commit ba03da2 into vitejs:main Jan 23, 2025
17 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/jsdom@26.1.0/node_modules/jsdom/lib/jsdom/virtual-console.js:29:53
 ❯ module.exports node_modules/.pnpm/jsdom@26.1.0/node_modules/jsdom/lib/jsdom/browser/not-implemented.js:12:26
 ❯ HTMLAudioElementImpl.load node_modules/.pnpm/jsdom@26.1.0/node_modules/jsdom/lib/jsdom/living/nodes/HTMLMediaElement-impl.js:112:5
 ❯ HTMLAudioElement.load node_modules/.pnpm/jsdom@26.1.0/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/jsdom@26.1.0/node_modules/jsdom/lib/jsdom/virtual-console.js:29:53
 ❯ module.exports node_modules/.pnpm/jsdom@26.1.0/node_modules/jsdom/lib/jsdom/browser/not-implemented.js:12:26
 ❯ HTMLAudioElementImpl.load node_modules/.pnpm/jsdom@26.1.0/node_modules/jsdom/lib/jsdom/living/nodes/HTMLMediaElement-impl.js:112:5
 ❯ HTMLAudioElement.load node_modules/.pnpm/jsdom@26.1.0/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/jsdom@26.1.0/node_modules/jsdom/lib/jsdom/virtual-console.js:29:53
 ❯ module.exports node_modules/.pnpm/jsdom@26.1.0/node_modules/jsdom/lib/jsdom/browser/not-implemented.js:12:26
 ❯ HTMLAudioElementImpl.load node_modules/.pnpm/jsdom@26.1.0/node_modules/jsdom/lib/jsdom/living/nodes/HTMLMediaElement-impl.js:112:5
 ❯ HTMLAudioElement.load node_modules/.pnpm/jsdom@26.1.0/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