From 4bf8e585cd68e0f5bb879a960073ed30791ebc37 Mon Sep 17 00:00:00 2001 From: Ryan Manuel Date: Thu, 14 Nov 2024 11:55:19 -0600 Subject: [PATCH] fix: ensure that each js file served up by vite dev server has an inline 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 | 4 + npm/vite-dev-server/src/plugins/sourcemap.ts | 33 +++--- .../test/plugins/sourcemap.spec.ts | 107 ++++++++++++++++++ 3 files changed, 130 insertions(+), 14 deletions(-) create mode 100644 npm/vite-dev-server/test/plugins/sourcemap.spec.ts diff --git a/cli/CHANGELOG.md b/cli/CHANGELOG.md index 51b0e3d745b5..87ef9285d4d5 100644 --- a/cli/CHANGELOG.md +++ b/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_ diff --git a/npm/vite-dev-server/src/plugins/sourcemap.ts b/npm/vite-dev-server/src/plugins/sourcemap.ts index 7111e2b54376..003e51cbcbd9 100644 --- a/npm/vite-dev-server/src/plugins/sourcemap.ts +++ b/npm/vite-dev-server/src/plugins/sourcemap.ts @@ -15,19 +15,16 @@ 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 @@ -35,8 +32,16 @@ export const CypressSourcemap = ( */ 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, diff --git a/npm/vite-dev-server/test/plugins/sourcemap.spec.ts b/npm/vite-dev-server/test/plugins/sourcemap.spec.ts new file mode 100644 index 000000000000..7270f854ed98 --- /dev/null +++ b/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') + } + }) +})