Skip to content

Commit

Permalink
Fix line numbers in case of commonJS file with no sourcemap
Browse files Browse the repository at this point in the history
Previously if someone imports or runs a commonJS file (one that won't
need to go through babel) we still need to wrap it in some code to run
it. But we don't have a sourcemap to update, unless it already had
one(internal babel adds it). This leads to all lines being off by 1.

This is fixed by updating the sourcemap in the other case.

Unfortunately short of generating an "identity" sourcemap and then
shifting it by 1 with adding `;` to the first mapping, this can't be
done.

The sourcemap implementation specifically wants to find each mapping, so
unless it can do that it will not do anything.

The remaining hack (adjusting any available sourcemap) still stands as
it also fixes the columns on the *first* line of the file. As well as
actually being better.
  • Loading branch information
mstoykov committed Apr 12, 2024
1 parent b6c47a1 commit e201243
Show file tree
Hide file tree
Showing 6 changed files with 53 additions and 12 deletions.
9 changes: 5 additions & 4 deletions js/bundle_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -91,7 +91,7 @@ func TestNewBundle(t *testing.T) {
_, err := getSimpleBundle(t, "/script.js", `throw new Error("aaaa");`)
exception := new(scriptExceptionError)
require.ErrorAs(t, err, &exception)
require.EqualError(t, err, "Error: aaaa\n\tat file:///script.js:2:7(3)\n")
require.EqualError(t, err, "Error: aaaa\n\tat file:///script.js:1:34(3)\n")
})
t.Run("InvalidExports", func(t *testing.T) {
t.Parallel()
Expand Down Expand Up @@ -164,13 +164,14 @@ func TestNewBundle(t *testing.T) {
// ES2015 modules are not supported
{
"Modules", "base", `export default function() {};`,
"file:///script.js: Line 2:1 Unexpected reserved word (and 2 more errors)",
"file:///script.js: Line 1:28 Unexpected reserved word (and 2 more errors)",
},
// BigInt is not supported
{
"BigInt", "base",
`module.exports.default = function() {}; BigInt(1231412444)`,
"ReferenceError: BigInt is not defined\n\tat file:///script.js:2:47(7)\n",
`module.exports.default = function() {};
BigInt(1231412444)`,
"ReferenceError: BigInt is not defined\n\tat file:///script.js:2:7(7)\n",
},
}

Expand Down
11 changes: 9 additions & 2 deletions js/compiler/compiler.go
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,7 @@ import (
"fmt"
"os"
"strconv"
"strings"
"sync"
"time"

Expand Down Expand Up @@ -207,8 +208,14 @@ func (c *Compiler) compileImpl(
) (*goja.Program, string, error) {
code := src
state := compilationState{srcMap: srcMap, compiler: c, wrapped: wrap}
if wrap { // the lines in the sourcemap (if available) will be fixed by increaseMappingsByOne
code = "(function(module, exports){\n" + code + "\n})\n"
if wrap {
conditionalNewLine := ""
if strings.Contains(code, "//# sourceMappingURL=") {
// the lines in the sourcemap (if available) will be fixed by increaseMappingsByOne
conditionalNewLine = "\n"
// if there is no sourcemap - bork only the first line of code, but leave the remaining ones.
}
code = "(function(module, exports){" + conditionalNewLine + code + "\n})\n"
}
opts := parser.WithDisableSourceMaps
if c.Options.SourceMapLoader != nil {
Expand Down
5 changes: 2 additions & 3 deletions js/compiler/compiler_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -84,7 +84,7 @@ func TestCompile(t *testing.T) {
src := `exports.d=1+(function() { return 2; })()`
pgm, code, err := c.Compile(src, "script.js", false)
require.NoError(t, err)
assert.Equal(t, "(function(module, exports){\nexports.d=1+(function() { return 2; })()\n})\n", code)
assert.Equal(t, "(function(module, exports){exports.d=1+(function() { return 2; })()\n})\n", code)
rt := goja.New()
v, err := rt.RunProgram(pgm)
require.NoError(t, err)
Expand Down Expand Up @@ -131,8 +131,7 @@ func TestCompile(t *testing.T) {
c.Options.CompatibilityMode = lib.CompatibilityModeExtended
pgm, code, err := c.Compile(`import "something";`, "script.js", false)
require.NoError(t, err)
assert.Equal(t, `(function(module, exports){
"use strict";require("something");
assert.Equal(t, `(function(module, exports){"use strict";require("something");
})
`, code)
var requireCalled bool
Expand Down
36 changes: 35 additions & 1 deletion js/initcontext_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -102,7 +102,7 @@ func TestRequire(t *testing.T) {
require.NoError(t, fsext.WriteFile(fs, "/file.js", []byte(`throw new Error("aaaa")`), 0o755))
_, err := getSimpleBundle(t, "/script.js", `import "/file.js"; export default function() {}`, fs)
assert.EqualError(t, err,
"Error: aaaa\n\tat file:///file.js:2:7(3)\n\tat go.k6.io/k6/js.(*requireImpl).require-fm (native)\n\tat file:///script.js:1:0(15)\n")
"Error: aaaa\n\tat file:///file.js:1:34(3)\n\tat go.k6.io/k6/js.(*requireImpl).require-fm (native)\n\tat file:///script.js:1:0(15)\n")
})

imports := map[string]struct {
Expand Down Expand Up @@ -576,6 +576,40 @@ export default function(){
require.Equal(t, exception.String(), "exception in line 2\n\tat f2 (file:///module1.js:2:4(2))\n\tat file:///script.js:5:4(3)\n")
}

func TestSourceMapsCJS(t *testing.T) {
t.Parallel()
fs := fsext.NewMemMapFs()
require.NoError(t, fsext.WriteFile(fs, "/module1.js", []byte(`
exports.f2 = function(){
throw "exception in line 2"
console.log("in f2")
}
exports.f1 = function(){
throw "exception in line 6"
console.log("in f1")
}
`[1:]), 0o644))
data := `
import * as module1 from "./module1.js"
export default function(){
// throw "exception in line 4"
module1.f2()
console.log("in default")
}
`[1:]
b, err := getSimpleBundle(t, "/script.js", data, fs)
require.NoError(t, err)

bi, err := b.Instantiate(context.Background(), 0)
require.NoError(t, err)
_, err = bi.getCallableExport(consts.DefaultFn)(goja.Undefined())
require.Error(t, err)
exception := new(goja.Exception)
require.ErrorAs(t, err, &exception)
require.Equal(t, exception.String(), "exception in line 2\n\tat file:///module1.js:2:5(2)\n\tat file:///script.js:5:4(3)\n")
}

func TestSourceMapsExternal(t *testing.T) {
t.Parallel()
fs := fsext.NewMemMapFs()
Expand Down
2 changes: 1 addition & 1 deletion js/runner_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -85,7 +85,7 @@ func TestRunnerNew(t *testing.T) {
t.Run("Invalid", func(t *testing.T) {
t.Parallel()
_, err := getSimpleRunner(t, "/script.js", `blarg`)
assert.EqualError(t, err, "ReferenceError: blarg is not defined\n\tat file:///script.js:2:1(1)\n")
assert.EqualError(t, err, "ReferenceError: blarg is not defined\n\tat file:///script.js:1:28(1)\n")
})
}

Expand Down
2 changes: 1 addition & 1 deletion js/summary_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -698,6 +698,6 @@ func TestExceptionInHandleSummaryFallsBackToTextSummary(t *testing.T) {
assert.Equal(t, 1, len(logErrors))
errMsg, err := logErrors[0].String()
require.NoError(t, err)
assert.Contains(t, errMsg, "\"Error: intentional error\\n\\tat file:///script.js:5:11(3)\\n")
assert.Contains(t, errMsg, "\"Error: intentional error\\n\\tat file:///script.js:4:11(3)\\n")
assert.Equal(t, logErrors[0].Data, logrus.Fields{"hint": "script exception"})
}

0 comments on commit e201243

Please sign in to comment.