From f3405894b4b91df086a9fa11bacd2d64b31e72fb Mon Sep 17 00:00:00 2001 From: Alex Eagle Date: Mon, 23 Sep 2019 12:34:37 -0700 Subject: [PATCH] feat(rollup): ensure that sourcemaps work end-to-end --- examples/BUILD.bazel | 1 + examples/webapp/BUILD.bazel | 15 +++- examples/webapp/differential_loading.bzl | 1 + examples/webapp/es5.babelrc | 1 + examples/webapp/index.js | 1 + examples/webapp/package.json | 12 +-- examples/webapp/sourcemaps.spec.js | 78 +++++++++++++++++++ packages/rollup/src/rollup_bundle.bzl | 13 ++-- packages/rollup/test/globals/BUILD.bazel | 2 +- .../test/integration_e2e_rollup/BUILD.bazel | 1 + packages/rollup/test/plugins/BUILD.bazel | 2 +- packages/rollup/test/sourcemaps/BUILD.bazel | 1 + .../rollup/test/version_stamp/BUILD.bazel | 3 +- 13 files changed, 112 insertions(+), 19 deletions(-) create mode 100644 examples/webapp/sourcemaps.spec.js diff --git a/examples/BUILD.bazel b/examples/BUILD.bazel index df022aa6f1..46526bf2f4 100644 --- a/examples/BUILD.bazel +++ b/examples/BUILD.bazel @@ -87,6 +87,7 @@ example_integration_test( example_integration_test( name = "examples_webapp", npm_packages = { + "//packages/jasmine:npm_package": "@bazel/jasmine", "//packages/protractor:npm_package": "@bazel/protractor", "//packages/rollup:npm_package": "@bazel/rollup", "//packages/terser:npm_package": "@bazel/terser", diff --git a/examples/webapp/BUILD.bazel b/examples/webapp/BUILD.bazel index abaf21ca4a..81cd1eb309 100644 --- a/examples/webapp/BUILD.bazel +++ b/examples/webapp/BUILD.bazel @@ -1,4 +1,5 @@ load("@npm//http-server:index.bzl", "http_server") +load("@npm_bazel_jasmine//:index.bzl", "jasmine_node_test") load("@npm_bazel_protractor//:index.bzl", "protractor_web_test_suite") load(":differential_loading.bzl", "differential_loading") @@ -30,10 +31,16 @@ protractor_web_test_suite( ], ) -# Just a dummy test so that we have a test target for //... on certain bazelci platforms with bazel_integration_test -sh_test( - name = "dummy_test", - srcs = ["dummy_test.sh"], +jasmine_node_test( + name = "test_sourcemaps", + srcs = ["sourcemaps.spec.js"], + deps = [ + ":app_chunks", + ":app_chunks.min", + ":app_chunks_es5", + ":app_chunks_es5.min", + "@npm//source-map", + ], ) # For testing from the root workspace of this repository with bazel_integration_test. diff --git a/examples/webapp/differential_loading.bzl b/examples/webapp/differential_loading.bzl index 7072fb96ac..c5a12be8de 100644 --- a/examples/webapp/differential_loading.bzl +++ b/examples/webapp/differential_loading.bzl @@ -12,6 +12,7 @@ def differential_loading(name, entry_point, srcs): rollup_bundle( name = name + "_chunks", srcs = srcs, + sourcemap = "inline", entry_points = { entry_point: "index", }, diff --git a/examples/webapp/es5.babelrc b/examples/webapp/es5.babelrc index a6cc19b07a..b0d24e0c1c 100644 --- a/examples/webapp/es5.babelrc +++ b/examples/webapp/es5.babelrc @@ -1,4 +1,5 @@ { + "sourceMaps": "inline", "presets": [ [ "@babel/preset-env", diff --git a/examples/webapp/index.js b/examples/webapp/index.js index f5ed3e7dbb..9a785844a4 100644 --- a/examples/webapp/index.js +++ b/examples/webapp/index.js @@ -2,6 +2,7 @@ import('./strings.en').then(m => { const msg = document.createElement('div'); msg.innerText = m.hello(); + // For sourcemap testing, keep this string literal on line 6 column 21 !! msg.className = 'ts1'; document.body.appendChild(msg); }); diff --git a/examples/webapp/package.json b/examples/webapp/package.json index 48fd45d02d..0ccaeb3f72 100644 --- a/examples/webapp/package.json +++ b/examples/webapp/package.json @@ -4,12 +4,14 @@ "@babel/cli": "^7.6.0", "@babel/core": "^7.6.0", "@babel/preset-env": "^7.6.0", - "@bazel/rollup": "latest", - "@bazel/terser": "latest", - "@bazel/protractor": "latest", + "@bazel/jasmine": "^0.37.1", + "@bazel/protractor": "file:/tmp/tmp-164023L7r1MgY0dmka", + "@bazel/rollup": "file:/tmp/tmp-164023xwqtzUemMOVT", + "@bazel/terser": "file:/tmp/tmp-164023K73Ev0H45uDb", "http-server": "^0.11.1", - "terser": "^4.3.1", - "rollup": "1.21.4" + "rollup": "1.21.4", + "source-map": "^0.7.3", + "terser": "^4.3.1" }, "scripts": { "test": "bazel test ..." diff --git a/examples/webapp/sourcemaps.spec.js b/examples/webapp/sourcemaps.spec.js new file mode 100644 index 0000000000..c6461c84da --- /dev/null +++ b/examples/webapp/sourcemaps.spec.js @@ -0,0 +1,78 @@ +// Ensure we have working sourcemaps when the app runs in a browser + +const fs = require('fs'); +const path = require('path'); +const sm = require('source-map'); + +const PRAGMA = '//# sourceMappingURL='; +const DATA = 'data:application/json;charset=utf-8;base64,'; + +function read(...f) { + return fs.readFileSync(require.resolve(path.join(__dirname, ...f)), 'utf-8'); +} + +function parseSourceMap(content) { + const sourcemapLine = content.split(/\r?\n/).find(l => l.startsWith(PRAGMA)); + if (!sourcemapLine) { + throw new Error(`no ${PRAGMA} found in ${content}`); + } + return JSON.parse(Buffer.from(sourcemapLine.slice(PRAGMA.length + DATA.length), 'base64')); +} + +function readSourceMap(...f) { + return JSON.parse(fs.readFileSync(require.resolve(path.join(__dirname, ...f)))); +} + +function find(text, s = 'ts1') { + const lines = text.split(/\r?\n/); + for (let line = 1; line <= lines.length; line++) { + const column = lines[line - 1].indexOf(s); + if (column >= 0) { + return {line, column}; + } + } +} + +function asserts(pos) { + // This doesn't work because the output dir is different from input + // so it actually starts with a bunch of '/../..' + // expect(pos.source).toBe('index.js'); + + expect(pos.source.endsWith('index.js')).toBeTruthy(); + expect(pos.line).toBe(6); // one-based + expect(pos.column).toBe(20); // zero-based +} + +describe('application sourcemaps in the browser', () => { + it('should work after rollup', async () => { + const content = read('app_chunks', 'index.js'); + const rawSourceMap = parseSourceMap(content); + await sm.SourceMapConsumer.with(rawSourceMap, null, consumer => { + asserts(consumer.originalPositionFor(find(content))); + }); + }); + + it('should work after terser', async () => { + const content = read('app_chunks.min', 'index.js'); + const rawSourceMap = readSourceMap('app_chunks.min', 'index.js.map'); + await sm.SourceMapConsumer.with(rawSourceMap, null, consumer => { + asserts(consumer.originalPositionFor(find(content))); + }); + }); + + it('should work after babel', async () => { + const content = read('app_chunks_es5', 'index.js'); + const rawSourceMap = parseSourceMap(content); + await sm.SourceMapConsumer.with(rawSourceMap, null, consumer => { + asserts(consumer.originalPositionFor(find(content))); + }); + }); + + it('should work after babel+terser', async () => { + const content = read('app_chunks_es5.min', 'index.js'); + const rawSourceMap = readSourceMap('app_chunks_es5.min', 'index.js.map'); + await sm.SourceMapConsumer.with(rawSourceMap, null, consumer => { + asserts(consumer.originalPositionFor(find(content))); + }); + }); +}); diff --git a/packages/rollup/src/rollup_bundle.bzl b/packages/rollup/src/rollup_bundle.bzl index feb89697c0..05f60dbc00 100644 --- a/packages/rollup/src/rollup_bundle.bzl +++ b/packages/rollup/src/rollup_bundle.bzl @@ -148,12 +148,13 @@ Otherwise, the outputs are assumed to be a single file. cfg = "host", default = "@npm//rollup/bin:rollup", ), - "sourcemap": attr.bool( - doc = """Whether to produce a .js.map output + "sourcemap": attr.string( + doc = """Whether to produce sourcemaps. Passed to the [`--sourcemap` option](https://github.com/rollup/rollup/blob/master/docs/999-big-list-of-options.md#outputsourcemap") in Rollup """, - default = True, + default = "inline", + values = ["inline", "true", "false"], ), "deps": attr.label_list( aspects = [module_mappings_aspect], @@ -218,7 +219,7 @@ def _rollup_outs(sourcemap, name, entry_point, entry_points, output_dir): fail("Multiple entry points require that output_dir be set") out = entry_point_outs[0] result[out] = out + ".js" - if sourcemap: + if sourcemap == "true": result[out + "_map"] = "%s.map" % result[out] return result @@ -273,8 +274,8 @@ def _rollup_bundle(ctx): # where the link is. args.add("--preserveSymlinks") - if (ctx.attr.sourcemap): - args.add("--sourcemap") + if (ctx.attr.sourcemap and ctx.attr.sourcemap != "false"): + args.add_all(["--sourcemap", ctx.attr.sourcemap]) if ctx.attr.globals: args.add("--external") diff --git a/packages/rollup/test/globals/BUILD.bazel b/packages/rollup/test/globals/BUILD.bazel index 2734259d00..08157d4c34 100644 --- a/packages/rollup/test/globals/BUILD.bazel +++ b/packages/rollup/test/globals/BUILD.bazel @@ -6,7 +6,7 @@ rollup_bundle( entry_point = "input.js", format = "umd", globals = {"some_global_var": "runtime_name_of_global_var"}, - sourcemap = False, + sourcemap = "false", ) golden_file_test( diff --git a/packages/rollup/test/integration_e2e_rollup/BUILD.bazel b/packages/rollup/test/integration_e2e_rollup/BUILD.bazel index 62aaafb402..14bbd1ed40 100644 --- a/packages/rollup/test/integration_e2e_rollup/BUILD.bazel +++ b/packages/rollup/test/integration_e2e_rollup/BUILD.bazel @@ -9,6 +9,7 @@ load("@npm_bazel_rollup//:index.from_src.bzl", "rollup_bundle") entry_point = "foo.js", format = format, globals = {"some_global_var": "runtime_name_of_global_var"}, + sourcemap = "true", deps = [ "//%s/fum:fumlib" % package_name(), "@npm//hello", diff --git a/packages/rollup/test/plugins/BUILD.bazel b/packages/rollup/test/plugins/BUILD.bazel index ebc47f90c8..6f04bdafa5 100644 --- a/packages/rollup/test/plugins/BUILD.bazel +++ b/packages/rollup/test/plugins/BUILD.bazel @@ -6,7 +6,7 @@ rollup_bundle( srcs = ["some.json"], config_file = "rollup.config.js", entry_point = "input.js", - sourcemap = False, + sourcemap = "false", ) golden_file_test( diff --git a/packages/rollup/test/sourcemaps/BUILD.bazel b/packages/rollup/test/sourcemaps/BUILD.bazel index b87d134877..836a6ba864 100644 --- a/packages/rollup/test/sourcemaps/BUILD.bazel +++ b/packages/rollup/test/sourcemaps/BUILD.bazel @@ -6,6 +6,7 @@ rollup_bundle( srcs = ["s.js"], # Use the desugared form to assert that it works entry_points = {"input.js": "bundle"}, + sourcemap = "true", ) jasmine_node_test( diff --git a/packages/rollup/test/version_stamp/BUILD.bazel b/packages/rollup/test/version_stamp/BUILD.bazel index 92b493d51d..aef2f58871 100644 --- a/packages/rollup/test/version_stamp/BUILD.bazel +++ b/packages/rollup/test/version_stamp/BUILD.bazel @@ -5,8 +5,7 @@ rollup_bundle( name = "version_stamp", config_file = "rollup.config.js", entry_point = "input.js", - # Disable sourcemap so we only get a single output - sourcemap = False, + sourcemap = "false", ) golden_file_test(