Skip to content

Commit b4149d8

Browse files
committed
fix(builtin): fix regression in 1.6.0 in linker linking root package when under runfiles
Resolves #1823 which had a repro https://github.com/purkhusid/ts-jest-repro. NB: The repo uses ts_library devmode UMD output so the relative require in the source code will be transformed to an absolute require which will include the workspace name. In 1.5.0: ``` .../sandbox/darwin-sandbox/23/execroot/jest_repro/bazel-out/darwin-fastbuild/bin/test.sh.runfiles/jest_repro/lib.test.js const lib_1 = require("jest_repro/lib"); ``` works as it finds ``` .../sandbox/darwin-sandbox/23/execroot/jest_repro/bazel-out/darwin-fastbuild/bin/test.sh.runfiles/jest_repro/node_modules/jest_repro/lib.js ``` where `test.sh.runfiles/jest_repro/node_modules/jest_repro` is a symlink to the root of the output tree setup by the linker: ``` lrwxr-xr-x 1 greg wheel 165 29 Apr 05:56 .../sandbox/darwin-sandbox/6/execroot/jest_repro/bazel-out/darwin-fastbuild/bin/test.sh.runfiles/jest_repro/node_modules/jest_repro -> .../sandbox/darwin-sandbox/6/execroot/jest_repro/bazel-out/darwin-fastbuild/bin/test.sh.runfiles/jest_repro ``` In 1.6.0: Bug introduced in #1850 (thanks for tracking down the culprit @alexeagle) where the root symlink is not created properly when under runfiles (such as a jest_test context). Fix is: ``` let runfilesPath = modulePath; if (runfilesPath.startsWith(`${bin}/`)) { runfilesPath = runfilesPath.slice(bin.length + 1); + } else if (runfilesPath === bin) { + runfilesPath = ''; } ``` in `link_node_modules.ts`. NB: This root workspace symlink will be removed in 2.0 as it causes other unrelated issues. PR for that change is in progress #1797. In 2.0, we recommend either using the prodmode esm output of ts_library for jest_test or switching to ts_project where the output flavor is controlled by your tsconfig. An example of using ts_library esm output of ts_library (output_group 'es6_sources') is `under /internal/linker/test/issue_1823_use_ts_library_esm`. It require babel-jest to transform the esm to commonjs. Alternately, you can control the 'es5_sources' module format of ts_library with the `devmode_module` but this can lead to other issues such as the output no longer being compatible with ts_devserver and karma_web_test which require named-UMD or named-AMD.
1 parent a167311 commit b4149d8

File tree

15 files changed

+346
-5
lines changed

15 files changed

+346
-5
lines changed

internal/linker/index.js

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -372,6 +372,9 @@ function main(args, runfiles) {
372372
if (runfilesPath.startsWith(`${bin}/`)) {
373373
runfilesPath = runfilesPath.slice(bin.length + 1);
374374
}
375+
else if (runfilesPath === bin) {
376+
runfilesPath = '';
377+
}
375378
const externalPrefix = 'external/';
376379
if (runfilesPath.startsWith(externalPrefix)) {
377380
runfilesPath = runfilesPath.slice(externalPrefix.length);

internal/linker/link_node_modules.ts

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -601,6 +601,8 @@ export async function main(args: string[], runfiles: Runfiles) {
601601
let runfilesPath = modulePath;
602602
if (runfilesPath.startsWith(`${bin}/`)) {
603603
runfilesPath = runfilesPath.slice(bin.length + 1);
604+
} else if (runfilesPath === bin) {
605+
runfilesPath = '';
604606
}
605607
const externalPrefix = 'external/';
606608
if (runfilesPath.startsWith(externalPrefix)) {
Lines changed: 22 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,22 @@
1+
load("@npm_bazel_typescript//:index.bzl", "ts_library")
2+
load(":ts_jest_test.bzl", "ts_jest_test")
3+
4+
ts_library(
5+
name = "lib",
6+
srcs = [
7+
"lib.ts",
8+
],
9+
deps = [
10+
],
11+
)
12+
13+
ts_jest_test(
14+
name = "test",
15+
srcs = [
16+
"lib.test.ts",
17+
],
18+
jest_config = "jest.config.js",
19+
deps = [
20+
":lib",
21+
],
22+
)
Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,3 @@
1+
module.exports = {
2+
testEnvironment: 'node',
3+
};
Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,7 @@
1+
import {doStuff} from './lib';
2+
3+
describe('doStuff', () => {
4+
it('should do some stuff', () => {
5+
expect(doStuff('boom')).toContain('boom');
6+
});
7+
});
Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,3 @@
1+
export function doStuff(a: string): string {
2+
return a
3+
}
Lines changed: 35 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,35 @@
1+
"""Simple macro around jest_test"""
2+
3+
load("@npm//jest-cli:index.bzl", _jest_test = "jest_test")
4+
load("@npm_bazel_typescript//:index.bzl", "ts_library")
5+
6+
def ts_jest_test(name, srcs, jest_config, deps = [], data = [], **kwargs):
7+
"""A macro around the autogenerated jest_test rule that takes typescript sources
8+
9+
Uses ts_library devmode UMD output"""
10+
11+
ts_library(
12+
name = "%s_ts" % name,
13+
srcs = srcs,
14+
data = data,
15+
deps = deps + ["@npm//@types/jest"],
16+
)
17+
native.filegroup(
18+
name = "%s_umd" % name,
19+
srcs = [":%s_ts" % name],
20+
output_group = "es5_sources",
21+
)
22+
23+
args = [
24+
"--no-cache",
25+
"--no-watchman",
26+
"--ci",
27+
]
28+
args.extend(["--config", "$(rootpath %s)" % jest_config])
29+
30+
_jest_test(
31+
name = name,
32+
data = [jest_config, ":%s_umd" % name] + deps + data,
33+
args = args,
34+
**kwargs
35+
)
Lines changed: 12 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,12 @@
1+
{
2+
"presets": [
3+
[
4+
"@babel/preset-env",
5+
{
6+
"targets": {
7+
"node": "current",
8+
},
9+
},
10+
],
11+
]
12+
}
Lines changed: 31 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,31 @@
1+
load("@npm_bazel_typescript//:index.bzl", "ts_library")
2+
load(":ts_jest_test.bzl", "ts_jest_test")
3+
4+
ts_library(
5+
name = "lib",
6+
srcs = [
7+
"lib.ts",
8+
],
9+
# NB: hacky hidden configuration setting so that es6_sources does not include tsickle
10+
# .externs.js outputs
11+
runtime = "nodejs",
12+
deps = [
13+
"@npm//@types/node",
14+
],
15+
)
16+
17+
ts_jest_test(
18+
name = "test",
19+
srcs = [
20+
"lib.test.ts",
21+
],
22+
data = [
23+
".babelrc",
24+
],
25+
jest_config = "jest.config.js",
26+
deps = [
27+
":lib",
28+
"@npm//@babel/preset-env",
29+
"@npm//babel-jest",
30+
],
31+
)
Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,6 @@
1+
module.exports = {
2+
testEnvironment: 'node',
3+
transform: {'^.+\\.mjs?$': 'babel-jest'},
4+
testMatch: ['**/?(*.)(spec|test).?(m)js?(x)'],
5+
moduleFileExtensions: ['js', 'mjs'],
6+
};

0 commit comments

Comments
 (0)