fix(ssrTransform): sourcemaps with multiple sources

[AriPerkkio]

Description

• Fixes SSR transform drops map.sources when multiple sources #17676

[stackblitz]

Run & review this pull request in StackBlitz Codeflow.

[AriPerkkio]

I'm not really sure what this comparison was here to check. In my repro it's failing with:

{
  sourcefile: './first.ts',
  escapedFilename: '/linux/fixtures/src/pre-bundle/bundle.js'
}

{
  sourcefile: './second.ts',
  escapedFilename: '/linux/fixtures/src/pre-bundle/bundle.js'
}

And removing this line doesn't break any test cases. Addition of this line goes as far as 2021, https://github.com/vitejs/vite/pull/2441/files#diff-071a32aedd2ea59472ebb69fb456e818b103d1a332da632e12c2d54395938ad1R421

[patak-dev]

That commit was based on source map for Svelte. @benmccann I see you reviewed the PR that merged this line here https://github.com/sveltejs/svelte/pull/5584/files#diff-8742d6c5cbad8aec49f195bc6d25b152ac0cd8180cb876c019f3b74736f9d62aR227. Maybe you have more context on why this was needed?

[benmccann]

I'm not much of a sourcemap expert. @milahu could know more

From my personal standpoint, I'd just suggest to make sure any changes don't break SvelteKit on the ecosystem CI as we have a small set of sourcemap tests there.

[patak-dev]

/ecosystem-ci run

[vite-ecosystem-ci]

📝 Ran ecosystem CI on 6fa6370: Open

suite	result	latest scheduled
analogjs	❌ failure	✅ success
astro	❌ failure	❌ failure
nuxt	❌ failure	❌ failure
sveltekit	❌ failure	✅ success
vite-plugin-react-pages	❌ failure	❌ failure
vite-plugin-vue	❌ failure	✅ success
vitest	❌ failure	❌ failure

✅ histoire, ladle, laravel, marko, previewjs, quasar, qwik, rakkas, remix, unocss, vike, vite-plugin-pwa, vite-plugin-react, vite-plugin-react-swc, vite-plugin-svelte, vite-setup-catalogue, vitepress

[AriPerkkio]

📝 Ran ecosystem CI on 6fa6370: Open

The vite-plugin-vue failures seem interesting: https://github.com/vitejs/vite-ecosystem-ci/actions/runs/9936723919/job/27445593714. If this change is indeed causing those, we'll need new test cases in Vite that fail as well.

Vitest failure is coming from Node 20.15.0 bug: nodejs/node#53648. Other failures seem unrelated, I think.

[benmccann]

It looks like the SvelteKit tests flaked out. Sorry about that. I'm not sure what caused it. We added a bunch of test improvements yesterday, but it seems they didn't help with that particular error. I'm not sure if it's possible to rerun just the SvelteKit tests? Probably @patak-dev or another maintainer would have to do it unless @dominikg knows if there's a bot command where you can leave a comment to do that

[patak-dev]

/ecosystem-ci run sveltekit

[vite-ecosystem-ci]

📝 Ran ecosystem CI on 6fa6370: Open

✅ sveltekit

[patak-dev]

We were just checking with @dominikg and everyone with triage access to the Vite repo should be able trigger ecosystem ci. @benmccann next time, you can try it for an individual project with

/ecosystem-ci run project

We can check if it isn't working for you then

[benmccann]

Thanks. When I said probably a maintainer would have to do it I was thinking of the rerun button in the GitHub UI. I didn't know about the command, so will keep that in mind for next time

[AriPerkkio]

Extracted the failing parts from vite-plugin-vue's failing test case. This PR is causing a real issue there.

// vite/packages/vite/repro.ts
// npx tsx repro.ts

import assert from 'node:assert'
import { combineSourcemaps } from './src/node/utils'

const out = combineSourcemaps(
  '/some-root-path/vite-plugin-vue/playground-temp/vue-sourcemap/SassWithImport.vue',
  [
    {
      version: 3,
      file: '/some-root-path/vite-plugin-vue/playground-temp/vue-sourcemap/SassWithImport.vue',
      mappings: 'AAAA;EACE;;;ACEF;EACE',
      names: [],
      ignoreList: [],
      sources: [
        '/some-root-path/vite-plugin-vue/playground-temp/vue-sourcemap/sassWithImportImported.sass',
        '/some-root-path/vite-plugin-vue/playground-temp/vue-sourcemap/SassWithImport.vue',
      ],
      sourcesContent: [null, null],
    },
    {
      version: 3,
      sources: [
        '/some-root-path/vite-plugin-vue/playground-temp/vue-sourcemap/SassWithImport.vue',
      ],
      names: [],
      mappings:
        ';AAMA,CAAC,CAAC,CAAC,CAAC,CAAC,CAAC,EAAE,CAAC,CAAC,CAAC,CAAC,CAAC,CAAC,CAAC,CAAC,CAAC,CAAC,CAAC,CAAC,CAAC,CAAC,CAAC,CAAC,CAAC,CAAC,CAAC,CAAC,CAAC,CAAC,CAAC,CAAC,CAAC;;AAEjC,CAAC,CAAC,CAAC,CAAC,CAAC,CAAC,CAAC,CAAC,CAAC,CAAC,CAAC,CAAC,CAAC,CAAC,CAAC,CAAC;EACd,CAAC,CAAC,CAAC,CAAC,CAAC,EAAE,CAAC,CAAC',
      file: '/some-root-path/vite-plugin-vue/playground-temp/vue-sourcemap/SassWithImport.vue',
      sourceRoot: '',
      sourcesContent: [
        '<template>\n  <p class="sass-with-import">&lt;sass&gt; with import</p>\n  <p class="sass-with-import-imported">&lt;sass&gt; with import (imported)</p>\n</template>\n\n<style lang="sass">\n@import \'./sassWithImportImported\'\n\n.sass-with-import\n  color: red\n</style>\n',
      ],
    },
  ],
)

console.log(out)

assert(out.sources.length === 2)

Results before:

sources: [
  '/some-root-path/vite-plugin-vue/playground-temp/vue-sourcemap/sassWithImportImported.sass',
  '/some-root-path/vite-plugin-vue/playground-temp/vue-sourcemap/SassWithImport.vue'
]

After:

sources: ['/some-root-path/vite-plugin-vue/playground-temp/vue-sourcemap/SassWithImport.vue'],

[AriPerkkio]

@patak-dev could you restart Ecosystem CI? I think I've got the vite-plugin-vue issue fixed now.

[patak-dev]

/ecosystem-ci run

[vite-ecosystem-ci]

📝 Ran ecosystem CI on 7a806b8: Open

suite	result	latest scheduled
astro	❌ failure	❌ failure
nuxt	❌ failure	❌ failure
vite-plugin-react-pages	❌ failure	❌ failure
vitest	❌ failure	❌ failure

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

[patak-dev]

Nice!

[hi-ogawa]

Looks great!

Previously in #15805, there was an issue due to multiple soruces where one is coming from esbuild define (e.g. "sources": ["<define:import.meta.env>", "...actual file..."]. I didn't know multiple sources support is possible, so I simply removed <define:...> part to make it work with combineSourcemaps.

With your fix on combineSourcemaps side, I wonder if "define hack" can be removed entirely.

[AriPerkkio]

With your fix on combineSourcemaps side, I wonder if "define hack" can be removed entirely.

Reverting the whole #15805 makes playground/js-sourcemap/__tests__/js-sourcemap.spec.ts fail.

Before (main): https://evanw.github.io/source-map-visualization/#MTI2AHZhciBlPXto...
After removing #15805: https://evanw.github.io/source-map-visualization/#MTI2AHZhciB...

Btw working on combineSourcemaps in Vite codebase is currently quite difficult as there are no direct unit tests for it. Testing it via plugin transforms is important, but debugging the changes done in combineSourcemaps is quite slow with those. Would be great to have clear test cases for combineSourcemaps where multiple maps are passed in and one comes out.

[hi-ogawa]

Hmm, I thought it might be fixed, but actually I wasn't sure how exactly define is affecting through the pipeline, so it's possible that combineSourcemaps wasn't the only cause. Thanks for checking it 🙏

[patak-dev]

Thanks for checking it too @hi-ogawa! Let's merge this one then.