From d0f6f2a728fb3d4d90289f25049384e0e6806855 Mon Sep 17 00:00:00 2001 From: BenceSzalai Date: Mon, 1 Aug 2022 00:59:21 +0200 Subject: [PATCH 1/7] fix: missing sourcemaps in js files with rewritten imports broke debugging (#7767) --- packages/vite/src/node/utils.ts | 19 ++++++++++++------ .../__tests__/js-sourcemap.spec.ts | 20 ++++++++++++++++++- playground/js-sourcemap/qux.js | 3 +++ .../__snapshots__/vue-sourcemap.spec.ts.snap | 5 ++--- 4 files changed, 37 insertions(+), 10 deletions(-) create mode 100644 playground/js-sourcemap/qux.js diff --git a/packages/vite/src/node/utils.ts b/packages/vite/src/node/utils.ts index 85f85adf3cf550..bad3da331f4e85 100644 --- a/packages/vite/src/node/utils.ts +++ b/packages/vite/src/node/utils.ts @@ -15,7 +15,7 @@ import type { DecodedSourceMap, RawSourceMap } from '@ampproject/remapping' import colors from 'picocolors' import debug from 'debug' import type { Alias, AliasOptions } from 'types/alias' -import type MagicString from 'magic-string' +import MagicString from 'magic-string' import type { TransformResult } from 'rollup' import { createFilter as _createFilter } from '@rollup/pluginutils' @@ -1102,19 +1102,26 @@ function normalizeSingleAlias({ /** * Transforms transpiled code result where line numbers aren't altered, - * so we can skip sourcemap generation during dev + * so we can skip full sourcemap generation during dev, however we still + * have to make a minimal sourcemap with the source property set to the + * original file so debuggers know which one to step into. */ export function transformStableResult( s: MagicString, id: string, config: ResolvedConfig ): TransformResult { + let map + if (config.command === 'build') { + map = config.build.sourcemap + ? s.generateMap({ hires: true, source: id }) + : null + } else { + map = new MagicString(s.toString()).generateMap({ hires: true, source: id }) + } return { code: s.toString(), - map: - config.command === 'build' && config.build.sourcemap - ? s.generateMap({ hires: true, source: id }) - : null + map } } diff --git a/playground/js-sourcemap/__tests__/js-sourcemap.spec.ts b/playground/js-sourcemap/__tests__/js-sourcemap.spec.ts index a7ecbdf145af4a..27dcfea6908c08 100644 --- a/playground/js-sourcemap/__tests__/js-sourcemap.spec.ts +++ b/playground/js-sourcemap/__tests__/js-sourcemap.spec.ts @@ -8,13 +8,31 @@ import { } from '~utils' if (!isBuild) { - test('js', async () => { + test('js without import', async () => { const res = await page.request.get(new URL('./foo.js', page.url()).href) const js = await res.text() const lines = js.split('\n') expect(lines[lines.length - 1].includes('//')).toBe(false) // expect no sourcemap }) + test('js', async () => { + const res = await page.request.get(new URL('./qux.js', page.url()).href) + const js = await res.text() + const map = extractSourcemap(js) + expect(formatSourcemapForSnapshot(map)).toMatchInlineSnapshot(` + { + "mappings": "AAAA,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;AACzB;AACA,CAAC,CAAC,CAAC,CAAC,CAAC,CAAC,CAAC,CAAC,CAAC,CAAC,CAAC,CAAC,CAAC,CAAC,CAAC,CAAC,CAAC,CAAC,CAAC,CAAC,CAAC,CAAC,CAAC,CAAC;", + "sources": [ + "/root/qux.js", + ], + "sourcesContent": [ + null, + ], + "version": 3, + } + `) + }) + test('ts', async () => { const res = await page.request.get(new URL('./bar.ts', page.url()).href) const js = await res.text() diff --git a/playground/js-sourcemap/qux.js b/playground/js-sourcemap/qux.js new file mode 100644 index 00000000000000..dc822ebd1b292f --- /dev/null +++ b/playground/js-sourcemap/qux.js @@ -0,0 +1,3 @@ +import foo from './foo' + +export const qux = 'qux' diff --git a/playground/vue-sourcemap/__tests__/__snapshots__/vue-sourcemap.spec.ts.snap b/playground/vue-sourcemap/__tests__/__snapshots__/vue-sourcemap.spec.ts.snap index d2600ee6edccce..74bb47eb2d2b05 100644 --- a/playground/vue-sourcemap/__tests__/__snapshots__/vue-sourcemap.spec.ts.snap +++ b/playground/vue-sourcemap/__tests__/__snapshots__/vue-sourcemap.spec.ts.snap @@ -189,8 +189,7 @@ exports[`serve:vue-sourcemap > less with additionalData > serve-less-with-additi exports[`serve:vue-sourcemap > no script > serve-no-script 1`] = ` { - "mappings": ";;;wBACE,oBAAwB,WAArB,aAAiB", - "sourceRoot": "", + "mappings": ";;;wBACE", "sources": [ "/root/NoScript.vue", ], @@ -206,7 +205,7 @@ exports[`serve:vue-sourcemap > no script > serve-no-script 1`] = ` exports[`serve:vue-sourcemap > no template > serve-no-template 1`] = ` { - "mappings": "AACA,CAAC,CAAC,CAAC,CAAC,CAAC,CAAC,CAAC,CAAC,CAAC,CAAC,CAAC,CAAC,CAAC,CAAC,CAAC,CAAC,CAAC,CAAC,CAAC,CAAC,CAAC;;;;;;AAGP;AACd,CAAC,CAAC,CAAC,CAAC,CAAC,CAAC,CAAC,CAAC,CAAC,CAAC,CAAC,CAAC,CAAC,CAAC,CAAC,CAAC,CAAC,CAAC,CAAC,CAAC;;;;;;;", + "mappings": "AACA,CAAC,CAAC,CAAC,CAAC,CAAC,CAAC,CAAC,CAAC,CAAC,CAAC,CAAC,CAAC,CAAC,CAAC,CAAC,CAAC,CAAC,CAAC,CAAC,CAAC,CAAC;;;;;;AAGP;AACd,CAAC,CAAC,CAAC,CAAC,CAAC,CAAC,CAAC,CAAC,CAAC,CAAC,CAAC,CAAC,CAAC,CAAC,CAAC,CAAC,CAAC,CAAC,CAAC,CAAC", "sources": [ "/root/NoTemplate.vue", ], From e7229d4da313663ad4ade28b4758824de422b53f Mon Sep 17 00:00:00 2001 From: BenceSzalai Date: Tue, 2 Aug 2022 00:32:30 +0200 Subject: [PATCH 2/7] perf: reduce overhead from dummy sourcemap generation Move the dummy sourcemap generation out of `transformStableResult()` so it has no impact on doing the transformations. Eliminate need for `highres: true` making generation faster and sourcemaps smaller when the mappings are superficial. --- .../vite/src/node/server/transformRequest.ts | 6 ++++++ packages/vite/src/node/utils.ts | 19 ++++++------------- .../__tests__/js-sourcemap.spec.ts | 2 +- .../__snapshots__/vue-sourcemap.spec.ts.snap | 5 +++-- 4 files changed, 16 insertions(+), 16 deletions(-) diff --git a/packages/vite/src/node/server/transformRequest.ts b/packages/vite/src/node/server/transformRequest.ts index 8714a547f60783..0bf9c367707665 100644 --- a/packages/vite/src/node/server/transformRequest.ts +++ b/packages/vite/src/node/server/transformRequest.ts @@ -5,6 +5,7 @@ import getEtag from 'etag' import * as convertSourceMap from 'convert-source-map' import type { SourceDescription, SourceMap } from 'rollup' import colors from 'picocolors' +import MagicString from 'magic-string' import type { ViteDevServer } from '..' import { cleanUrl, @@ -249,6 +250,11 @@ async function loadAndTransform( isDebug && debugTransform(`${timeFrom(transformStart)} ${prettyUrl}`) code = transformResult.code! map = transformResult.map + + // To enable debugging create a sourcemap for known modified JS files without one: + if (!map && mod.file && mod.type === 'js' && code !== originalCode) { + map = new MagicString(code).generateMap({ source: mod.file }) + } } if (map && mod.file) { diff --git a/packages/vite/src/node/utils.ts b/packages/vite/src/node/utils.ts index bad3da331f4e85..85f85adf3cf550 100644 --- a/packages/vite/src/node/utils.ts +++ b/packages/vite/src/node/utils.ts @@ -15,7 +15,7 @@ import type { DecodedSourceMap, RawSourceMap } from '@ampproject/remapping' import colors from 'picocolors' import debug from 'debug' import type { Alias, AliasOptions } from 'types/alias' -import MagicString from 'magic-string' +import type MagicString from 'magic-string' import type { TransformResult } from 'rollup' import { createFilter as _createFilter } from '@rollup/pluginutils' @@ -1102,26 +1102,19 @@ function normalizeSingleAlias({ /** * Transforms transpiled code result where line numbers aren't altered, - * so we can skip full sourcemap generation during dev, however we still - * have to make a minimal sourcemap with the source property set to the - * original file so debuggers know which one to step into. + * so we can skip sourcemap generation during dev */ export function transformStableResult( s: MagicString, id: string, config: ResolvedConfig ): TransformResult { - let map - if (config.command === 'build') { - map = config.build.sourcemap - ? s.generateMap({ hires: true, source: id }) - : null - } else { - map = new MagicString(s.toString()).generateMap({ hires: true, source: id }) - } return { code: s.toString(), - map + map: + config.command === 'build' && config.build.sourcemap + ? s.generateMap({ hires: true, source: id }) + : null } } diff --git a/playground/js-sourcemap/__tests__/js-sourcemap.spec.ts b/playground/js-sourcemap/__tests__/js-sourcemap.spec.ts index 27dcfea6908c08..e31252c8148334 100644 --- a/playground/js-sourcemap/__tests__/js-sourcemap.spec.ts +++ b/playground/js-sourcemap/__tests__/js-sourcemap.spec.ts @@ -21,7 +21,7 @@ if (!isBuild) { const map = extractSourcemap(js) expect(formatSourcemapForSnapshot(map)).toMatchInlineSnapshot(` { - "mappings": "AAAA,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;AACzB;AACA,CAAC,CAAC,CAAC,CAAC,CAAC,CAAC,CAAC,CAAC,CAAC,CAAC,CAAC,CAAC,CAAC,CAAC,CAAC,CAAC,CAAC,CAAC,CAAC,CAAC,CAAC,CAAC,CAAC,CAAC;", + "mappings": "AAAA;AACA;AACA;", "sources": [ "/root/qux.js", ], diff --git a/playground/vue-sourcemap/__tests__/__snapshots__/vue-sourcemap.spec.ts.snap b/playground/vue-sourcemap/__tests__/__snapshots__/vue-sourcemap.spec.ts.snap index 74bb47eb2d2b05..d2600ee6edccce 100644 --- a/playground/vue-sourcemap/__tests__/__snapshots__/vue-sourcemap.spec.ts.snap +++ b/playground/vue-sourcemap/__tests__/__snapshots__/vue-sourcemap.spec.ts.snap @@ -189,7 +189,8 @@ exports[`serve:vue-sourcemap > less with additionalData > serve-less-with-additi exports[`serve:vue-sourcemap > no script > serve-no-script 1`] = ` { - "mappings": ";;;wBACE", + "mappings": ";;;wBACE,oBAAwB,WAArB,aAAiB", + "sourceRoot": "", "sources": [ "/root/NoScript.vue", ], @@ -205,7 +206,7 @@ exports[`serve:vue-sourcemap > no script > serve-no-script 1`] = ` exports[`serve:vue-sourcemap > no template > serve-no-template 1`] = ` { - "mappings": "AACA,CAAC,CAAC,CAAC,CAAC,CAAC,CAAC,CAAC,CAAC,CAAC,CAAC,CAAC,CAAC,CAAC,CAAC,CAAC,CAAC,CAAC,CAAC,CAAC,CAAC,CAAC;;;;;;AAGP;AACd,CAAC,CAAC,CAAC,CAAC,CAAC,CAAC,CAAC,CAAC,CAAC,CAAC,CAAC,CAAC,CAAC,CAAC,CAAC,CAAC,CAAC,CAAC,CAAC,CAAC", + "mappings": "AACA,CAAC,CAAC,CAAC,CAAC,CAAC,CAAC,CAAC,CAAC,CAAC,CAAC,CAAC,CAAC,CAAC,CAAC,CAAC,CAAC,CAAC,CAAC,CAAC,CAAC,CAAC;;;;;;AAGP;AACd,CAAC,CAAC,CAAC,CAAC,CAAC,CAAC,CAAC,CAAC,CAAC,CAAC,CAAC,CAAC,CAAC,CAAC,CAAC,CAAC,CAAC,CAAC,CAAC,CAAC;;;;;;;", "sources": [ "/root/NoTemplate.vue", ], From ba7879b82ef5b49d0f31871d1c276f44701d20e5 Mon Sep 17 00:00:00 2001 From: BenceSzalai Date: Tue, 2 Aug 2022 16:25:13 +0200 Subject: [PATCH 3/7] fix: inject sourcesContent into dummy sourcemap to make browser debugging is possible --- packages/vite/src/node/server/transformRequest.ts | 6 +++++- playground/js-sourcemap/__tests__/js-sourcemap.spec.ts | 5 ++++- 2 files changed, 9 insertions(+), 2 deletions(-) diff --git a/packages/vite/src/node/server/transformRequest.ts b/packages/vite/src/node/server/transformRequest.ts index 0bf9c367707665..b7f3667d536b78 100644 --- a/packages/vite/src/node/server/transformRequest.ts +++ b/packages/vite/src/node/server/transformRequest.ts @@ -259,7 +259,11 @@ async function loadAndTransform( if (map && mod.file) { map = (typeof map === 'string' ? JSON.parse(map) : map) as SourceMap - if (map.mappings && !map.sourcesContent) { + if ( + map.mappings && + (!map.sourcesContent || + (map.sourcesContent as Array).includes(null)) + ) { await injectSourcesContent(map, mod.file, logger) } } diff --git a/playground/js-sourcemap/__tests__/js-sourcemap.spec.ts b/playground/js-sourcemap/__tests__/js-sourcemap.spec.ts index e31252c8148334..9669f3c3ae07c7 100644 --- a/playground/js-sourcemap/__tests__/js-sourcemap.spec.ts +++ b/playground/js-sourcemap/__tests__/js-sourcemap.spec.ts @@ -26,7 +26,10 @@ if (!isBuild) { "/root/qux.js", ], "sourcesContent": [ - null, + "import foo from './foo' + +export const qux = 'qux' +", ], "version": 3, } From 7e4cb2a9564e39bd8eae1be7d928f84a11eb793a Mon Sep 17 00:00:00 2001 From: BenceSzalai Date: Thu, 4 Aug 2022 19:15:24 +0200 Subject: [PATCH 4/7] test: fix broken import in dummy sourcemap test The issue did not affect the testing as the loaded JS file was not executed, only fixed for the sake of correctness. --- playground/js-sourcemap/__tests__/js-sourcemap.spec.ts | 2 +- playground/js-sourcemap/qux.js | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/playground/js-sourcemap/__tests__/js-sourcemap.spec.ts b/playground/js-sourcemap/__tests__/js-sourcemap.spec.ts index 9669f3c3ae07c7..2029e7f1f321f8 100644 --- a/playground/js-sourcemap/__tests__/js-sourcemap.spec.ts +++ b/playground/js-sourcemap/__tests__/js-sourcemap.spec.ts @@ -26,7 +26,7 @@ if (!isBuild) { "/root/qux.js", ], "sourcesContent": [ - "import foo from './foo' + "import { foo } from './foo' export const qux = 'qux' ", diff --git a/playground/js-sourcemap/qux.js b/playground/js-sourcemap/qux.js index dc822ebd1b292f..1536173b2f96d0 100644 --- a/playground/js-sourcemap/qux.js +++ b/playground/js-sourcemap/qux.js @@ -1,3 +1,3 @@ -import foo from './foo' +import { foo } from './foo' export const qux = 'qux' From 6061436add0e8a43525608d190a24cad379933d3 Mon Sep 17 00:00:00 2001 From: BenceSzalai Date: Thu, 3 Nov 2022 03:10:39 +0100 Subject: [PATCH 5/7] fix: exclude HMR responses from dummy sourcemap generation Compatibility was broken with fix https://github.com/vitejs/vite/pull/9914 but also we don't need and don't want to add dummy sourcemaps to HMR JS requests. --- packages/vite/src/node/server/transformRequest.ts | 12 ++++++++++-- 1 file changed, 10 insertions(+), 2 deletions(-) diff --git a/packages/vite/src/node/server/transformRequest.ts b/packages/vite/src/node/server/transformRequest.ts index 252551e4f38a42..6cb541df63c36b 100644 --- a/packages/vite/src/node/server/transformRequest.ts +++ b/packages/vite/src/node/server/transformRequest.ts @@ -253,8 +253,16 @@ async function loadAndTransform( code = transformResult.code! map = transformResult.map - // To enable debugging create a sourcemap for known modified JS files without one: - if (!map && mod.file && mod.type === 'js' && code !== originalCode) { + // To enable IDE debugging add a minimal sourcemap for modified non-HMR JS files without one: + if ( + !map && + mod.file && + mod.type === 'js' && + code !== originalCode && + !code.startsWith( + 'import { createHotContext as __vite__createHotContext } from' + ) + ) { map = new MagicString(code).generateMap({ source: mod.file }) } } From ef84eb12b6f2cf5b94efafe06084e6bca88fc249 Mon Sep 17 00:00:00 2001 From: BenceSzalai Date: Mon, 14 Nov 2022 15:03:41 +0100 Subject: [PATCH 6/7] feat: implement more structured check for css and other special responses Based on the [suggestion](https://github.com/vitejs/vite/pull/9476#discussion_r1021319370) of [sapphi-red](https://github.com/sapphi-red) --- packages/vite/src/node/server/transformRequest.ts | 7 ++++--- 1 file changed, 4 insertions(+), 3 deletions(-) diff --git a/packages/vite/src/node/server/transformRequest.ts b/packages/vite/src/node/server/transformRequest.ts index 6cb541df63c36b..0f09aae07cb543 100644 --- a/packages/vite/src/node/server/transformRequest.ts +++ b/packages/vite/src/node/server/transformRequest.ts @@ -19,6 +19,8 @@ import { } from '../utils' import { checkPublicFile } from '../plugins/asset' import { getDepsOptimizer } from '../optimizer' +import { isCSSRequest } from '../plugins/css' +import { SPECIAL_QUERY_RE } from '../constants' import { injectSourcesContent } from './sourcemap' import { isFileServingAllowed } from './middlewares/static' @@ -259,9 +261,8 @@ async function loadAndTransform( mod.file && mod.type === 'js' && code !== originalCode && - !code.startsWith( - 'import { createHotContext as __vite__createHotContext } from' - ) + !isCSSRequest(id) && // skip CSS : #9914 + !SPECIAL_QUERY_RE.test(id) // skip special requests ) { map = new MagicString(code).generateMap({ source: mod.file }) } From 15dc03ec91806d2b84f176765289f72d015e5aa4 Mon Sep 17 00:00:00 2001 From: sapphi-red Date: Sun, 20 Nov 2022 19:14:16 +0900 Subject: [PATCH 7/7] chore: tweak --- packages/vite/src/node/server/transformRequest.ts | 5 ++--- 1 file changed, 2 insertions(+), 3 deletions(-) diff --git a/packages/vite/src/node/server/transformRequest.ts b/packages/vite/src/node/server/transformRequest.ts index 0f09aae07cb543..d36fd9cb15aa17 100644 --- a/packages/vite/src/node/server/transformRequest.ts +++ b/packages/vite/src/node/server/transformRequest.ts @@ -255,14 +255,13 @@ async function loadAndTransform( code = transformResult.code! map = transformResult.map - // To enable IDE debugging add a minimal sourcemap for modified non-HMR JS files without one: + // To enable IDE debugging, add a minimal sourcemap for modified JS files without one if ( !map && mod.file && mod.type === 'js' && code !== originalCode && - !isCSSRequest(id) && // skip CSS : #9914 - !SPECIAL_QUERY_RE.test(id) // skip special requests + !(isCSSRequest(id) && !SPECIAL_QUERY_RE.test(id)) // skip CSS : #9914 ) { map = new MagicString(code).generateMap({ source: mod.file }) }