Skip to content

Commit

Permalink
fix: breakpoints in JS not working (#13514)
Browse files Browse the repository at this point in the history
  • Loading branch information
sapphi-red committed Aug 22, 2023
1 parent c39e6c1 commit 0156bd2
Show file tree
Hide file tree
Showing 10 changed files with 88 additions and 43 deletions.
2 changes: 1 addition & 1 deletion packages/vite/rollup.config.ts
Original file line number Diff line number Diff line change
Expand Up @@ -195,7 +195,7 @@ function createCjsConfig(isProduction: boolean) {
...Object.keys(pkg.dependencies),
...(isProduction ? [] : Object.keys(pkg.devDependencies)),
],
plugins: [...createNodePlugins(false, false, false), bundleSizeLimit(123)],
plugins: [...createNodePlugins(false, false, false), bundleSizeLimit(155)],
})
}

Expand Down
4 changes: 2 additions & 2 deletions packages/vite/src/node/server/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -235,7 +235,7 @@ export interface ViteDevServer {
*/
ssrTransform(
code: string,
inMap: SourceMap | null,
inMap: SourceMap | { mappings: '' } | null,
url: string,
originalCode?: string,
): Promise<TransformResult | null>
Expand Down Expand Up @@ -385,7 +385,7 @@ export async function _createServer(
resolvedUrls: null, // will be set on listen
ssrTransform(
code: string,
inMap: SourceMap | null,
inMap: SourceMap | { mappings: '' } | null,
url: string,
originalCode = code,
) {
Expand Down
2 changes: 1 addition & 1 deletion packages/vite/src/node/server/middlewares/indexHtml.ts
Original file line number Diff line number Diff line change
Expand Up @@ -282,7 +282,7 @@ const devHtmlHook: IndexHtmlTransformHook = async (
const result = await server!.pluginContainer.transform(code, mod.id!)
let content = ''
if (result) {
if (result.map) {
if (result.map && 'version' in result.map) {
if (result.map.mappings) {
await injectSourcesContent(
result.map,
Expand Down
45 changes: 29 additions & 16 deletions packages/vite/src/node/server/pluginContainer.ts
Original file line number Diff line number Diff line change
Expand Up @@ -132,7 +132,7 @@ export interface PluginContainer {
inMap?: SourceDescription['map']
ssr?: boolean
},
): Promise<{ code: string; map: SourceMap | null }>
): Promise<{ code: string; map: SourceMap | { mappings: '' } | null }>
load(
id: string,
options?: {
Expand Down Expand Up @@ -481,7 +481,7 @@ export async function createPluginContainer(
typeof err.loc?.column === 'number'
) {
const rawSourceMap = ctx._getCombinedSourcemap()
if (rawSourceMap) {
if (rawSourceMap && 'version' in rawSourceMap) {
const traced = new TraceMap(rawSourceMap as any)
const { source, line, column } = originalPositionFor(traced, {
line: Number(err.loc.line),
Expand Down Expand Up @@ -525,7 +525,7 @@ export async function createPluginContainer(
originalCode: string
originalSourcemap: SourceMap | null = null
sourcemapChain: NonNullable<SourceDescription['map']>[] = []
combinedMap: SourceMap | null = null
combinedMap: SourceMap | { mappings: '' } | null = null

constructor(filename: string, code: string, inMap?: SourceMap | string) {
super()
Expand All @@ -540,7 +540,7 @@ export async function createPluginContainer(
}
}

_getCombinedSourcemap(createIfNull = false) {
_getCombinedSourcemap() {
if (
debugSourcemapCombine &&
debugSourcemapCombineFilter &&
Expand All @@ -553,12 +553,26 @@ export async function createPluginContainer(
}

let combinedMap = this.combinedMap
// { mappings: '' }
if (
combinedMap &&
!('version' in combinedMap) &&
combinedMap.mappings === ''
) {
this.sourcemapChain.length = 0
return combinedMap
}

for (let m of this.sourcemapChain) {
if (typeof m === 'string') m = JSON.parse(m)
if (!('version' in (m as SourceMap))) {
// { mappings: '' }
if ((m as SourceMap).mappings === '') {
combinedMap = { mappings: '' }
break
}
// empty, nullified source map
combinedMap = this.combinedMap = null
this.sourcemapChain.length = 0
combinedMap = null
break
}
if (!combinedMap) {
Expand All @@ -570,15 +584,6 @@ export async function createPluginContainer(
]) as SourceMap
}
}
if (!combinedMap) {
return createIfNull
? new MagicString(this.originalCode).generateMap({
includeContent: true,
hires: 'boundary',
source: cleanUrl(this.filename),
})
: null
}
if (combinedMap !== this.combinedMap) {
this.combinedMap = combinedMap
this.sourcemapChain.length = 0
Expand All @@ -587,7 +592,15 @@ export async function createPluginContainer(
}

getCombinedSourcemap() {
return this._getCombinedSourcemap(true) as SourceMap
const map = this._getCombinedSourcemap()
if (!map || (!('version' in map) && map.mappings === '')) {
return new MagicString(this.originalCode).generateMap({
includeContent: true,
hires: 'boundary',
source: cleanUrl(this.filename),
})
}
return map
}
}

Expand Down
16 changes: 14 additions & 2 deletions packages/vite/src/node/server/send.ts
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,8 @@ import type {
} from 'node:http'
import getEtag from 'etag'
import type { SourceMap } from 'rollup'
import MagicString from 'magic-string'
import { removeTimestampQuery } from '../utils'
import { getCodeWithSourcemap } from './sourcemap'

const alias: Record<string, string | undefined> = {
Expand All @@ -18,7 +20,7 @@ export interface SendOptions {
etag?: string
cacheControl?: string
headers?: OutgoingHttpHeaders
map?: SourceMap | null
map?: SourceMap | { mappings: '' } | null
}

export function send(
Expand Down Expand Up @@ -56,10 +58,20 @@ export function send(
}

// inject source map reference
if (map && map.mappings) {
if (map && 'version' in map && map.mappings) {
if (type === 'js' || type === 'css') {
content = getCodeWithSourcemap(type, content.toString(), map)
}
} else {
if (type === 'js' && (!map || map.mappings !== '')) {
const urlWithoutTimestamp = removeTimestampQuery(req.url!)
const ms = new MagicString(content.toString())
content = getCodeWithSourcemap(
type,
content.toString(),
ms.generateMap({ source: urlWithoutTimestamp, hires: 'boundary' }),
)
}
}

res.statusCode = 200
Expand Down
32 changes: 20 additions & 12 deletions packages/vite/src/node/server/transformRequest.ts
Original file line number Diff line number Diff line change
Expand Up @@ -31,7 +31,7 @@ const debugCache = createDebugger('vite:cache')

export interface TransformResult {
code: string
map: SourceMap | null
map: SourceMap | { mappings: '' } | null
etag?: string
deps?: string[]
dynamicDeps?: string[]
Expand Down Expand Up @@ -286,15 +286,23 @@ async function loadAndTransform(
map = transformResult.map
}

if (map && mod.file) {
map = (typeof map === 'string' ? JSON.parse(map) : map) as SourceMap
if (map.mappings) {
await injectSourcesContent(map, mod.file, logger)
let normalizedMap: SourceMap | { mappings: '' } | null
if (typeof map === 'string') {
normalizedMap = JSON.parse(map)
} else if (map) {
normalizedMap = map as SourceMap | { mappings: '' }
} else {
normalizedMap = null
}

if (normalizedMap && 'version' in normalizedMap && mod.file) {
if (normalizedMap.mappings) {
await injectSourcesContent(normalizedMap, mod.file, logger)
}

const sourcemapPath = `${mod.file}.map`
applySourcemapIgnoreList(
map,
normalizedMap,
sourcemapPath,
config.server.sourcemapIgnoreList,
logger,
Expand All @@ -303,16 +311,16 @@ async function loadAndTransform(
if (path.isAbsolute(mod.file)) {
for (
let sourcesIndex = 0;
sourcesIndex < map.sources.length;
sourcesIndex < normalizedMap.sources.length;
++sourcesIndex
) {
const sourcePath = map.sources[sourcesIndex]
const sourcePath = normalizedMap.sources[sourcesIndex]
if (sourcePath) {
// Rewrite sources to relative paths to give debuggers the chance
// to resolve and display them in a meaningful way (rather than
// with absolute paths).
if (path.isAbsolute(sourcePath)) {
map.sources[sourcesIndex] = path.relative(
normalizedMap.sources[sourcesIndex] = path.relative(
path.dirname(mod.file),
sourcePath,
)
Expand All @@ -326,12 +334,12 @@ async function loadAndTransform(

const result =
ssr && !server.config.experimental.skipSsrTransform
? await server.ssrTransform(code, map as SourceMap, url, originalCode)
? await server.ssrTransform(code, normalizedMap, url, originalCode)
: ({
code,
map,
map: normalizedMap,
etag: getEtag(code, { weak: true }),
} as TransformResult)
} satisfies TransformResult)

// Only cache the result if the module wasn't invalidated while it was
// being processed, so it is re-processed next time if it is stale
Expand Down
2 changes: 1 addition & 1 deletion packages/vite/src/node/ssr/ssrModuleLoader.ts
Original file line number Diff line number Diff line change
Expand Up @@ -202,7 +202,7 @@ async function instantiateModule(
}

let sourceMapSuffix = ''
if (result.map) {
if (result.map && 'version' in result.map) {
const moduleSourceMap = Object.assign({}, result.map, {
// currently we need to offset the line
// https://github.com/nodejs/node/issues/43047#issuecomment-1180632750
Expand Down
13 changes: 9 additions & 4 deletions packages/vite/src/node/ssr/ssrTransform.ts
Original file line number Diff line number Diff line change
Expand Up @@ -38,7 +38,7 @@ const hashbangRE = /^#!.*\n/

export async function ssrTransform(
code: string,
inMap: SourceMap | null,
inMap: SourceMap | { mappings: '' } | null,
url: string,
originalCode: string,
options?: TransformOptions,
Expand All @@ -51,7 +51,7 @@ export async function ssrTransform(

async function ssrTransformJSON(
code: string,
inMap: SourceMap | null,
inMap: SourceMap | { mappings: '' } | null,
): Promise<TransformResult> {
return {
code: code.replace('export default', `${ssrModuleExportsKey}.default =`),
Expand All @@ -63,7 +63,7 @@ async function ssrTransformJSON(

async function ssrTransformScript(
code: string,
inMap: SourceMap | null,
inMap: SourceMap | { mappings: '' } | null,
url: string,
originalCode: string,
): Promise<TransformResult | null> {
Expand Down Expand Up @@ -275,7 +275,12 @@ async function ssrTransformScript(
})

let map = s.generateMap({ hires: 'boundary' })
if (inMap && inMap.mappings && inMap.sources.length > 0) {
if (
inMap &&
inMap.mappings &&
'sources' in inMap &&
inMap.sources.length > 0
) {
map = combineSourcemaps(url, [
{
...map,
Expand Down
3 changes: 1 addition & 2 deletions playground/css-sourcemap/__tests__/css-sourcemap.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -82,8 +82,7 @@ describe.runIf(isServe)('serve', () => {
new URL('./linked-with-import.css', page.url()).href,
)
const content = await res.text()
const lines = content.trim().split('\n')
expect(lines[lines.length - 1]).not.toMatch(/^\/\/#/)
expect(content).not.toMatch('//#s*sourceMappingURL')
},
)

Expand Down
12 changes: 10 additions & 2 deletions playground/js-sourcemap/__tests__/js-sourcemap.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -13,8 +13,16 @@ if (!isBuild) {
test('js', 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
const map = extractSourcemap(js)
expect(formatSourcemapForSnapshot(map)).toMatchInlineSnapshot(`
{
"mappings": "AAAA,MAAM,CAAC,KAAK,CAAC,GAAG,CAAC,CAAC,CAAC,CAAC,GAAG,CAAC;",
"sources": [
"/foo.js",
],
"version": 3,
}
`)
})

test('ts', async () => {
Expand Down

0 comments on commit 0156bd2

Please sign in to comment.