Skip to content

Commit

Permalink
fix source mappings for external import paths
Browse files Browse the repository at this point in the history
  • Loading branch information
@evanw
evanw committed Mar 18, 2021
1 parent 64c7c09 commit ab7e61c
Show file tree
Hide file tree
Showing 3 changed files with 73 additions and 22 deletions.
34 changes: 17 additions & 17 deletions internal/js_parser/js_parser.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -11119,7 +11119,7 @@ func (p *parser) visitExprInOut(expr js_ast.Expr, in exprIn) (js_ast.Expr, exprO

importRecordIndex := p.addImportRecord(ast.ImportDynamic, arg.Loc, js_lexer.UTF16ToString(str.Value))
p.importRecordsForCurrentPart = append(p.importRecordsForCurrentPart, importRecordIndex)
return js_ast.Expr{Loc: arg.Loc, Data: &js_ast.EImport{
return js_ast.Expr{Loc: expr.Loc, Data: &js_ast.EImport{
Expr: arg,
ImportRecordIndex: ast.MakeIndex32(importRecordIndex),
LeadingInteriorComments: e.LeadingInteriorComments,
Expand Down Expand Up @@ -11163,33 +11163,33 @@ func (p *parser) visitExprInOut(expr js_ast.Expr, in exprIn) (js_ast.Expr, exprO
// correctly, and you need a string literal to get an import record.
if !p.options.outputFormat.KeepES6ImportExportSyntax() {
var then js_ast.Expr
value := p.callRuntime(arg.Loc, "__toModule", []js_ast.Expr{{Loc: arg.Loc, Data: &js_ast.ECall{
Target: js_ast.Expr{Loc: arg.Loc, Data: &js_ast.EIdentifier{Ref: p.requireRef}},
value := p.callRuntime(arg.Loc, "__toModule", []js_ast.Expr{{Loc: expr.Loc, Data: &js_ast.ECall{
Target: js_ast.Expr{Loc: expr.Loc, Data: &js_ast.EIdentifier{Ref: p.requireRef}},
Args: []js_ast.Expr{arg},
}}})
body := js_ast.FnBody{Loc: arg.Loc, Stmts: []js_ast.Stmt{{Loc: arg.Loc, Data: &js_ast.SReturn{Value: &value}}}}
body := js_ast.FnBody{Loc: expr.Loc, Stmts: []js_ast.Stmt{{Loc: expr.Loc, Data: &js_ast.SReturn{Value: &value}}}}
if p.options.unsupportedJSFeatures.Has(compat.Arrow) {
then = js_ast.Expr{Loc: arg.Loc, Data: &js_ast.EFunction{Fn: js_ast.Fn{Body: body}}}
then = js_ast.Expr{Loc: expr.Loc, Data: &js_ast.EFunction{Fn: js_ast.Fn{Body: body}}}
} else {
then = js_ast.Expr{Loc: arg.Loc, Data: &js_ast.EArrow{Body: body, PreferExpr: true}}
then = js_ast.Expr{Loc: expr.Loc, Data: &js_ast.EArrow{Body: body, PreferExpr: true}}
}
return js_ast.Expr{Loc: arg.Loc, Data: &js_ast.ECall{
Target: js_ast.Expr{Loc: arg.Loc, Data: &js_ast.EDot{
Target: js_ast.Expr{Loc: arg.Loc, Data: &js_ast.ECall{
Target: js_ast.Expr{Loc: arg.Loc, Data: &js_ast.EDot{
Target: js_ast.Expr{Loc: arg.Loc, Data: &js_ast.EIdentifier{Ref: p.makePromiseRef()}},
return js_ast.Expr{Loc: expr.Loc, Data: &js_ast.ECall{
Target: js_ast.Expr{Loc: expr.Loc, Data: &js_ast.EDot{
Target: js_ast.Expr{Loc: expr.Loc, Data: &js_ast.ECall{
Target: js_ast.Expr{Loc: expr.Loc, Data: &js_ast.EDot{
Target: js_ast.Expr{Loc: expr.Loc, Data: &js_ast.EIdentifier{Ref: p.makePromiseRef()}},
Name: "resolve",
NameLoc: arg.Loc,
NameLoc: expr.Loc,
}},
}},
Name: "then",
NameLoc: arg.Loc,
NameLoc: expr.Loc,
}},
Args: []js_ast.Expr{then},
}}
}

return js_ast.Expr{Loc: arg.Loc, Data: &js_ast.EImport{
return js_ast.Expr{Loc: expr.Loc, Data: &js_ast.EImport{
Expr: arg,
LeadingInteriorComments: e.LeadingInteriorComments,
}}
Expand Down Expand Up @@ -11365,7 +11365,7 @@ func (p *parser) visitExprInOut(expr js_ast.Expr, in exprIn) (js_ast.Expr, exprO
// We don't want to spend time scanning the required files if they will
// never be used.
if p.isControlFlowDead {
return js_ast.Expr{Loc: arg.Loc, Data: &js_ast.ENull{}}
return js_ast.Expr{Loc: expr.Loc, Data: &js_ast.ENull{}}
}

importRecordIndex := p.addImportRecord(ast.ImportRequire, arg.Loc, js_lexer.UTF16ToString(str.Value))
Expand All @@ -11374,7 +11374,7 @@ func (p *parser) visitExprInOut(expr js_ast.Expr, in exprIn) (js_ast.Expr, exprO

// Create a new expression to represent the operation
p.ignoreUsage(p.requireRef)
return js_ast.Expr{Loc: arg.Loc, Data: &js_ast.ERequire{ImportRecordIndex: importRecordIndex}}
return js_ast.Expr{Loc: expr.Loc, Data: &js_ast.ERequire{ImportRecordIndex: importRecordIndex}}
}

if !omitWarnings {
Expand All @@ -11384,7 +11384,7 @@ func (p *parser) visitExprInOut(expr js_ast.Expr, in exprIn) (js_ast.Expr, exprO
}

// Otherwise just return a clone of the "require()" call
return js_ast.Expr{Loc: arg.Loc, Data: &js_ast.ECall{
return js_ast.Expr{Loc: expr.Loc, Data: &js_ast.ECall{
Target: js_ast.Expr{Loc: e.Target.Loc, Data: &js_ast.EIdentifier{Ref: id.Ref}},
Args: []js_ast.Expr{arg},
}}
Expand Down
2 changes: 2 additions & 0 deletions internal/js_printer/js_printer.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -1226,6 +1226,7 @@ func (p *printer) printRequireOrImportExpr(importRecordIndex uint32, leadingInte
}
p.printIndent()
}
p.addSourceMapping(record.Range.Loc)
p.printQuotedUTF8(record.Path.Text, true /* allowBacktick */)
if len(leadingInteriorComments) > 0 {
p.printNewline()
Expand Down Expand Up @@ -1270,6 +1271,7 @@ func (p *printer) printRequireOrImportExpr(importRecordIndex uint32, leadingInte
p.print("()")
} else {
p.print("require(")
p.addSourceMapping(record.Range.Loc)
p.printQuotedUTF8(record.Path.Text, true /* allowBacktick */)
p.print(")")
}
Expand Down
59 changes: 54 additions & 5 deletions scripts/verify-source-map.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -262,7 +262,30 @@ const toSearchComplex = {
'forceUpdate': '../../node_modules/react/cjs/react.production.min.js',
};

async function check(kind, testCase, toSearch, { flags, entryPoints, crlf }) {
const testCaseDynamicImport = {
'entry.js': `
const then = (x) => console.log("imported", x);
console.log([import("./ext/a.js").then(then), import("./ext/ab.js").then(then), import("./ext/abc.js").then(then)]);
console.log([import("./ext/abc.js").then(then), import("./ext/ab.js").then(then), import("./ext/a.js").then(then)]);
`,
'ext/a.js': `
export default 'a'
`,
'ext/ab.js': `
export default 'ab'
`,
'ext/abc.js': `
export default 'abc'
`,
}

const toSearchDynamicImport = {
'./ext/a.js': 'entry.js',
'./ext/ab.js': 'entry.js',
'./ext/abc.js': 'entry.js',
};

async function check(kind, testCase, toSearch, { flags, entryPoints, crlf, followUpFlags = [] }) {
let failed = 0

try {
Expand All @@ -273,7 +296,7 @@ async function check(kind, testCase, toSearch, { flags, entryPoints, crlf }) {
}
}

const tempDir = path.join(testDir, '' + tempDirCount++)
const tempDir = path.join(testDir, `${kind}-${tempDirCount++}`)
await fs.mkdir(tempDir, { recursive: true })

for (const name in testCase) {
Expand Down Expand Up @@ -326,7 +349,7 @@ async function check(kind, testCase, toSearch, { flags, entryPoints, crlf }) {
const { source, line, column } = map.originalPositionFor({ line: outLine, column: outColumn })

const inSource = isStdin ? '<stdin>' : toSearch[id];
recordCheck(source === inSource, `expected: ${inSource} observed: ${source}`)
recordCheck(source === inSource, `expected source: ${inSource}, observed source: ${source}`)

const inJs = map.sourceContentFor(source)
let inIndex = inJs.indexOf(`"${id}"`)
Expand All @@ -338,7 +361,21 @@ async function check(kind, testCase, toSearch, { flags, entryPoints, crlf }) {

const expected = JSON.stringify({ source, line: inLine, column: inColumn })
const observed = JSON.stringify({ source, line, column })
recordCheck(expected === observed, `expected: ${expected} observed: ${observed}`)
recordCheck(expected === observed, `expected original position: ${expected}, observed original position: ${observed}`)

// Also check the reverse mapping
const positions = map.allGeneratedPositionsFor({ source, line: inLine, column: inColumn })
recordCheck(positions.length > 0, `expected generated positions: 1, observed generated positions: ${positions.length}`)
let found = false
for (const { line, column } of positions) {
if (line === outLine && column === outColumn) {
found = true
break
}
}
const expectedPosition = JSON.stringify({ line: outLine, column: outColumn })
const observedPositions = JSON.stringify(positions)
recordCheck(found, `expected generated position: ${expectedPosition}, observed generated positions: ${observedPositions}`)
}
}

Expand Down Expand Up @@ -381,7 +418,7 @@ async function check(kind, testCase, toSearch, { flags, entryPoints, crlf }) {
order === 1 ? `import './${fileToTest}'; import './extra.js'` :
order === 2 ? `import './extra.js'; import './${fileToTest}'` :
`import './${fileToTest}'`)
await execFileAsync(esbuildPath, [nestedEntry, '--bundle', '--outfile=' + path.join(tempDir, 'out2.js'), '--sourcemap'], { cwd: testDir })
await execFileAsync(esbuildPath, [nestedEntry, '--bundle', '--outfile=' + path.join(tempDir, 'out2.js'), '--sourcemap'].concat(followUpFlags), { cwd: testDir })

const out2Js = await fs.readFile(path.join(tempDir, 'out2.js'), 'utf8')
recordCheck(out2Js.includes(`//# sourceMappingURL=out2.js.map\n`), `.js file links to .js.map`)
Expand Down Expand Up @@ -479,6 +516,18 @@ async function main() {
entryPoints: ['entry.js'],
crlf,
}),
check('dynamic-import' + suffix, testCaseDynamicImport, toSearchDynamicImport, {
flags: flags.concat('--outfile=out.js', '--bundle', '--external:./ext/*', '--format=esm'),
entryPoints: ['entry.js'],
crlf,
followUpFlags: ['--external:./ext/*', '--format=esm'],
}),
check('dynamic-require' + suffix, testCaseDynamicImport, toSearchDynamicImport, {
flags: flags.concat('--outfile=out.js', '--bundle', '--external:./ext/*', '--format=cjs'),
entryPoints: ['entry.js'],
crlf,
followUpFlags: ['--external:./ext/*', '--format=cjs'],
}),
)
}
}
Expand Down

0 comments on commit ab7e61c

Please sign in to comment.