fix: ensure that each js file served up by vite dev server has an inl…

…ine sourcemap (#30606)

* fix: ensure that each js file served up by vite dev server has an inline sourcemap

* refactor

* refactor with more understanding

* add changelog

* PR comments and other tweaks

* add comments

* add comments

* Apply suggestions from code review

cli/CHANGELOG.md

@@ -7,6 +7,10 @@ _Released 11/19/2024 (PENDING)_
 
 - Updated the protocol to be able to flex logic based on project config. Addresses [#30560](https://github.com/cypress-io/cypress/issues/30560).
 
+**Bugfixes:**
+
+- Fixed an issue where some JS assets were not properly getting sourcemaps included with the vite dev server if they had a cache busting query parameter in the URL. Fixed some scenarios to ensure that the sourcemaps that were included by the vite dev server were inlined. Addressed in [#30606](https://github.com/cypress-io/cypress/pull/30606).
+
 ## 13.15.2
 
 _Released 11/5/2024_

npm/vite-dev-server/src/plugins/sourcemap.ts

@@ -15,28 +15,33 @@ export const CypressSourcemap = (
     enforce: 'post',
     transform (code, id, options?) {
       try {
-        if (/\.js$/i.test(id) && !/\/\/# sourceMappingURL=/i.test(code)) {
-          /*
-          The Vite dev server and plugins automatically generate sourcemaps for most files, but they are
-          only included in the served files if any transpilation actually occurred (JSX, TS, etc). This
-          means that files that are "pure" JS won't include sourcemaps, so JS spec files that don't require
-          transpilation won't get code frames in the runner.
-
-          A sourcemap for these files is generated (just not served) which is in effect an "identity"
-          sourcemap mapping 1-to-1 to the output file. We can grab this and pass it along as a sourcemap
-          we want Vite to embed into the output, giving Cypress a sourcemap to use for codeFrame lookups.
-          @see https://rollupjs.org/guide/en/#thisgetcombinedsourcemap
+        // Remove query parameters from the id. This is necessary because some files
+        // have a cache buster query parameter (e.g. `?v=12345`)
+        const queryParameterLessId = id.split('?')[0]
 
-          We utilize a 'post' plugin here and manually append the sourcemap content to the code. We *should*
+        // Check if the file has a JavaScript extension and does not have an inlined sourcemap
+        if (/\.(js|jsx|ts|tsx|vue|svelte|mjs|cjs)$/i.test(queryParameterLessId) && !/\/\/# sourceMappingURL=data/i.test(code)) {
+          /*
+          The Vite dev server and plugins automatically generate sourcemaps for most files, but there are
+          some files that don't have sourcemaps generated for them or are not inlined. We utilize a 'post' plugin
+          here and manually append the sourcemap content to the code in these cases. We *should*
           be able to pass the sourcemap along using the `map` attribute in the return value, but Babel-based
           plugins don't work with this which causes sourcemaps to break for files that go through common
           plugins like `@vitejs/plugin-react`. By manually appending the sourcemap at this point in time
           Babel-transformed files end up with correct sourcemaps.
           */
 
           const sourcemap = this.getCombinedSourcemap()
-
-          code += `\n//# sourceMappingURL=${sourcemap.toUrl()}`
+          const sourcemapUrl = sourcemap.toUrl()
+
+          if (/\/\/# sourceMappingURL=/i.test(code)) {
+            // If the code already has a sourceMappingURL, it is not an inlined sourcemap
+            // and we should replace it with the new sourcemap
+            code = code.replace(/\/\/# sourceMappingURL=(.*)$/m, `//# sourceMappingURL=${sourcemapUrl}`)
+          } else {
+            // If the code does not have a sourceMappingURL, we should append the new sourcemap
+            code += `\n//# sourceMappingURL=${sourcemapUrl}`
+          }
 
           return {
             code,

npm/vite-dev-server/test/plugins/sourcemap.spec.ts

@@ -0,0 +1,107 @@
+import { Plugin } from 'vite-5'
+import { ViteDevServerConfig } from '../../src/devServer'
+import { Vite } from '../../src/getVite'
+import { CypressSourcemap } from '../../src/plugins'
+import Chai, { expect } from 'chai'
+import SinonChai from 'sinon-chai'
+import sinon from 'sinon'
+
+Chai.use(SinonChai)
+
+describe('sourcemap plugin', () => {
+  ['js', 'jsx', 'ts', 'tsx', 'vue', 'svelte', 'mjs', 'cjs'].forEach((ext) => {
+    it(`should append sourcemap to the code if sourceMappingURL is not present for files with extension ${ext}`, () => {
+      const code = 'console.log("hello world")'
+      const id = `test.js`
+      const options = {} as ViteDevServerConfig
+      const vite = {} as Vite
+      const plugin = CypressSourcemap(options, vite) as Plugin & { getCombinedSourcemap: () => { toUrl: () => string } }
+
+      plugin.getCombinedSourcemap = () => {
+        return {
+          toUrl: () => 'data:application/json;base64,eyJ2ZXJzaW9uIjozfQ==',
+        }
+      }
+
+      expect(plugin.name).to.equal('cypress:sourcemap')
+      expect(plugin.enforce).to.equal('post')
+
+      if (plugin.transform instanceof Function) {
+        const result = plugin.transform.call(plugin, code, id)
+
+        expect(result.code).to.eq('console.log("hello world")\n//# sourceMappingURL=data:application/json;base64,eyJ2ZXJzaW9uIjozfQ==')
+      } else {
+        throw new Error('transform is not a function')
+      }
+    })
+
+    it(`should replace sourceMappingURL with sourcemap and handle query parameters for files with extension ${ext}`, () => {
+      const code = 'console.log("hello world")\n//# sourceMappingURL=old-url'
+      const id = `test.js?v=12345`
+      const options = {} as ViteDevServerConfig
+      const vite = {} as Vite
+      const plugin = CypressSourcemap(options, vite) as Plugin & { getCombinedSourcemap: () => { toUrl: () => string } }
+
+      plugin.getCombinedSourcemap = () => {
+        return {
+          toUrl: () => 'data:application/json;base64,eyJ2ZXJzaW9uIjozfQ==',
+        }
+      }
+
+      expect(plugin.name).to.equal('cypress:sourcemap')
+      expect(plugin.enforce).to.equal('post')
+
+      if (plugin.transform instanceof Function) {
+        const result = plugin.transform.call(plugin, code, id)
+
+        expect(result.code).to.eq('console.log("hello world")\n//# sourceMappingURL=data:application/json;base64,eyJ2ZXJzaW9uIjozfQ==')
+      } else {
+        throw new Error('transform is not a function')
+      }
+    })
+  })
+
+  it('should not append sourcemap to the code if sourceMappingURL is already present', () => {
+    const code = 'console.log("hello world")\n//# sourceMappingURL=data:application/json;base64,eyJ2ZXJzaW9uIjozfQ=='
+    const id = `test.js`
+    const options = {} as ViteDevServerConfig
+    const vite = {} as Vite
+    const plugin = CypressSourcemap(options, vite) as Plugin & { getCombinedSourcemap: () => { toUrl: () => string } }
+
+    plugin.getCombinedSourcemap = sinon.stub()
+
+    expect(plugin.name).to.equal('cypress:sourcemap')
+    expect(plugin.enforce).to.equal('post')
+
+    if (plugin.transform instanceof Function) {
+      const result = plugin.transform.call(plugin, code, id)
+
+      expect(result).to.be.undefined
+      expect(plugin.getCombinedSourcemap).not.to.have.been.called
+    } else {
+      throw new Error('transform is not a function')
+    }
+  })
+
+  it('should not append sourcemap to the code if the file is not a js, jsx, ts, tsx, vue, mjs, or cjs file', () => {
+    const code = 'console.log("hello world")'
+    const id = `test.css`
+    const options = {} as ViteDevServerConfig
+    const vite = {} as Vite
+    const plugin = CypressSourcemap(options, vite) as Plugin & { getCombinedSourcemap: () => { toUrl: () => string } }
+
+    plugin.getCombinedSourcemap = sinon.stub()
+
+    expect(plugin.name).to.equal('cypress:sourcemap')
+    expect(plugin.enforce).to.equal('post')
+
+    if (plugin.transform instanceof Function) {
+      const result = plugin.transform.call(plugin, code, id)
+
+      expect(result).to.be.undefined
+      expect(plugin.getCombinedSourcemap).not.to.have.been.called
+    } else {
+      throw new Error('transform is not a function')
+    }
+  })
+})