Worker mode holds bad state for missing dependency

[alexeagle]

Thanks to @petebacondarwin for isolating this bug.

Create a vanilla TS project containing:

/BUILD.bazel

package(default_visibility=["//visibility:public"])
load("@build_bazel_rules_typescript//:defs.bzl", "ts_library")

ts_library(
    name = "my_code",
    srcs = glob(["*.ts"]),
    deps = [
        #"@npm//source-map",
        #"@npm//@types/source-map",
    ],
)

thing.ts

import {} from 'source-map';

package.json

{
  "name": "repro_ts_typecheck",
  "version": "1.0.0",
  "main": "index.js",
  "license": "MIT",
  "devDependencies": {
    "@bazel/typescript": "^0.22.1",
    "@types/source-map": "^0.5.7",
    "typescript": "^3.2.2"
  },
  "dependencies": {
    "source-map": "^0.7.3"
  }
}

Then build:

$ bazel build :my_code --strategy=TypeScriptCompile=worker
INFO: Invocation ID: 5ac6555c-36fe-40f8-925c-61ea9430e779
INFO: Analysed target //:my_code (1 packages loaded, 3 targets configured).
INFO: Found 1 target...
ERROR: /usr/local/google/home/alexeagle/Projects/repro_ts_typecheck/BUILD.bazel:4:1: Compiling TypeScript (devmode) //:my_code failed (Exit 1)

WARNING: your tsconfig.json file specifies options which are overridden by Bazel:
 - compilerOptions.target and compilerOptions.module are controlled by downstream dependencies, such as ts_devserver


error TS2688: Cannot find type definition file for 'source-map'.
thing.ts:1:16 - error TS2307: Cannot find module 'source-map'.

1 import {} from 'source-map';
                 ~~~~~~~~~~~~

Target //:my_code failed to build

So far this is WAI because the needed deps are commented out. So uncomment them, which should make the build succeed. However, it still does this:

$ cat BUILD.bazel 
package(default_visibility=["//visibility:public"])
load("@build_bazel_rules_typescript//:defs.bzl", "ts_library")

ts_library(
    name = "my_code",
    srcs = glob(["*.ts"]),
    deps = [
        "@npm//source-map",
        "@npm//@types/source-map",
    ],
)
alexeagle@alexeagle:~/Projects/repro_ts_typecheck$ bazel build :my_code --strategy=TypeScriptCompile=worker
INFO: Invocation ID: 14af237c-9084-4322-87fe-2766e3ff8abd
INFO: Analysed target //:my_code (5 packages loaded, 31 targets configured).
INFO: Found 1 target...
ERROR: /usr/local/google/home/alexeagle/Projects/repro_ts_typecheck/BUILD.bazel:4:1: Compiling TypeScript (devmode) //:my_code failed (Exit 1)

WARNING: your tsconfig.json file specifies options which are overridden by Bazel:
 - compilerOptions.target and compilerOptions.module are controlled by downstream dependencies, such as ts_devserver


error TS2688: Cannot find type definition file for 'source-map'.
thing.ts:1:16 - error TS2307: Cannot find module 'source-map'.

1 import {} from 'source-map';
                 ~~~~~~~~~~~~

Target //:my_code failed to build

But if you make no changes and immediately re-build without using the persistent worker, it succeeds.

$ bazel build :my_code
INFO: Invocation ID: 7e5222cf-ace3-476f-9f83-389c4e85911e
INFO: Analysed target //:my_code (0 packages loaded, 0 targets configured).
INFO: Found 1 target...
INFO: From Compiling TypeScript (devmode) //:my_code:

WARNING: your tsconfig.json file specifies options which are overridden by Bazel:
 - compilerOptions.target and compilerOptions.module are controlled by downstream dependencies, such as ts_devserver


Target //:my_code up-to-date:
  bazel-bin/thing.d.ts

That means the persistent worker got stuck in a bad state.

[alexeagle]

I confirmed that commenting out the programCache, by setting oldProgram to undefined rather than this value

rules_typescript/internal/tsc_wrapped/tsc_wrapped.ts

Line 201 in 4a087c0

const oldProgram = cache.getProgram(bazelOpts.target);

causes the issue to no longer be reproducible

[alexeagle]

This is not reproducible in google3. This is because the google3 generated tsconfig lists all files that were in the deps so the options passed to ts.createProgram are different. This discards the oldProgram so typescript tries again.

Under Bazel, though, we filter out fine-grained npm deps from being passed to the underlying implementation:

rules_typescript/internal/build_defs.bzl

Line 243 in 0f7c6f6

deps = [d for d in ctx.attr.deps if not NodeModuleInfo in d],

to avoid an error

ERROR: /home/alexeagle/.cache/bazel/_bazel_alexeagle/b35e8144baca7ebd0f491e6d98c75ccb/external/build_bazel_rules_typescript/internal/BUILD.bazel:42:1: in ts_library rule @build_bazel_rules_typescript//internal:tsc_wrapped: 
Traceback (most recent call last):
        File "/home/alexeagle/.cache/bazel/_bazel_alexeagle/b35e8144baca7ebd0f491e6d98c75ccb/external/build_bazel_rules_typescript/internal/BUILD.bazel", line 42
                ts_library(name = 'tsc_wrapped')
        File "/home/alexeagle/.cache/bazel/_bazel_alexeagle/b35e8144baca7ebd0f491e6d98c75ccb/external/build_bazel_rules_typescript/internal/build_defs.bzl", line 228, in _ts_library_impl
                compile_ts(ctx, is_library = True, deps = ctx.a..., <3 more arguments>)
        File "/home/alexeagle/.cache/bazel/_bazel_alexeagle/b35e8144baca7ebd0f491e6d98c75ccb/external/build_bazel_rules_typescript/internal/common/compilation.bzl", line 223, in compile_ts
                assert_js_or_typescript_deps(ctx, deps)
        File "/home/alexeagle/.cache/bazel/_bazel_alexeagle/b35e8144baca7ebd0f491e6d98c75ccb/external/build_bazel_rules_typescript/internal/common/compilation.bzl", line 72, in assert_js_or_typescript_deps
                fail(("%s is neither a TypeScript nor...)))
@npm//node_modules/@types/node:node__pkg is neither a TypeScript nor a JS producing rule.
Dependencies must be ts_library

Instead we just stitch the files into the action inputs here

rules_typescript/internal/build_defs.bzl

Lines 102 to 106 in 0f7c6f6

	# Also include files from npm fine grained deps as action_inputs.
	# These deps are identified by the NodeModuleInfo provider.
	for d in ctx.attr.deps:
	if NodeModuleInfo in d:
	action_inputs.extend(_filter_ts_inputs(d.files))

That means that the files[] in the generated tsconfig is unchanged, and so the call to ts.createProgram has no changes from the previous run, so the same ts.Program is reused and so it has the same failing diagnostic.

The right fix is for the target @npm//node_modules/@types/node:node__pkg to be a typescript typings Provider so that it can be passed through as a dep, and remove these two bits of logic from build_defs.bzl

[gregmagolan]

Should be resolved by bazel-contrib/rules_nodejs#726 which was just merged. Will leave this open until this is verified after the next rules_nodejs release since CI does not test for this.