Skip to content

Commit

Permalink
fix(typescript): exclude typescript lib declarations in
Browse files Browse the repository at this point in the history
...node_module_library transitive_declarations as the including /node_modules/typescript/lib/lib.*.d.ts is controlled by compilerOptions libs. Including all of the can cause typings conflicts.

Test coverage also added for this that verifies that typescript/lib/typescript.d.ts and typescript/lib/tsserverlibrary.d.ts are available since we must exclude other typescript/lib/ .d.ts files which are controlled by the compilerOptions.libs config. Also verify that libs not specified such as lib.webworkder.d.ts in compileOptions.libs config are not included in the compilation unit. See #875 for more details.
  • Loading branch information
gregmagolan authored and alexeagle committed Jun 19, 2019
1 parent 22e61f8 commit 3d55b41
Show file tree
Hide file tree
Showing 6 changed files with 30 additions and 11 deletions.
5 changes: 4 additions & 1 deletion e2e/ts_library/foobar/BUILD.bazel
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,10 @@ ts_library(

ts_library(
name = "foo_ts_library",
srcs = ["foo.ts"],
srcs = [
"conflict.d.ts",
"foo.ts",
],
tsconfig = ":tsconfig-foo.json",
deps = [
":types",
Expand Down
12 changes: 11 additions & 1 deletion e2e/ts_library/foobar/bar.ts
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,16 @@ import {a} from 'e2e_ts_library/generated_ts/foo';
// Repro for #31, should automatically discover @types/node
import * as fs from 'fs';
import {cool} from 'some-lib';
// Verify that typescript/lib/typescript.d.ts and typescript/lib/tsserverlibrary.d.ts
// are available since we must exclude all other typescript/lib/*.d.ts which are
// controlled by the compilerOptions.libs config.
// See https://github.com/bazelbuild/rules_nodejs/pull/875 for more details.
import * as ts from 'typescript';
import * as tss from 'typescript/lib/tsserverlibrary';

import('./foo').then(({greeter}) => {console.log(Greeter, fs, cool, ts, greeter, a);});
import('./foo').then(({greeter}) => {console.log(Greeter, fs, cool, ts, greeter, a);});

const useTssType: tss.server.Project[] = [];
if (useTssType) {
console.log('foobar');
}
4 changes: 4 additions & 0 deletions e2e/ts_library/foobar/conflict.d.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,4 @@
// Test that we can declare an interface named Client
// which would conflict with typescript/lib/lib.webworker.d.ts if that
// lib was included but should work if that lib is not
declare class Client { id: string; }
2 changes: 1 addition & 1 deletion e2e/ts_library/package.json
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,7 @@
"@types/hammerjs": "2.0.35",
"@types/jasmine": "3.3.9",
"@types/node": "11.11.2",
"typescript": "^3.3.1",
"typescript": "~3.4.2",
"tsickle": "0.33.1",
"zone.js": "0.8.26"
},
Expand Down
12 changes: 6 additions & 6 deletions e2e/ts_library/yarn.lock
Original file line number Diff line number Diff line change
Expand Up @@ -3,14 +3,14 @@


"@bazel/jasmine@file:../../dist/npm_bazel_jasmine":
version "0.30.1-2-g5cc5c06"
version "0.32.0-4-gf2ff790"
dependencies:
jasmine "~3.3.1"
jasmine-core "~3.3.0"
v8-coverage "1.0.9"

"@bazel/typescript@file:../../dist/npm_bazel_typescript":
version "0.30.1-2-g5cc5c06"
version "0.32.0-4-gf2ff790"
dependencies:
protobufjs "6.8.8"
semver "5.6.0"
Expand Down Expand Up @@ -829,10 +829,10 @@ tsutils@2.27.2:
dependencies:
tslib "^1.8.1"

typescript@^3.3.1:
version "3.3.3333"
resolved "https://registry.yarnpkg.com/typescript/-/typescript-3.3.3333.tgz#171b2c5af66c59e9431199117a3bcadc66fdcfd6"
integrity sha512-JjSKsAfuHBE/fB2oZ8NxtRTk5iGcg6hkYXMnZ3Wc+b2RSqejEqTaem11mHASMnFilHrax3sLK0GDzcJrekZYLw==
typescript@~3.4.2:
version "3.4.5"
resolved "https://registry.yarnpkg.com/typescript/-/typescript-3.4.5.tgz#2d2618d10bb566572b8d7aad5180d84257d70a99"
integrity sha512-YycBxUb49UUhdNMU5aJ7z5Ej2XGmaIBL0x34vZ82fn3hGvD+bgrMrVDpatgz2f7YxUMJxMkbWxJZeAvDxVe7Vw==

uglify-js@^3.1.4:
version "3.5.3"
Expand Down
6 changes: 4 additions & 2 deletions internal/npm_install/node_module_library.bzl
Original file line number Diff line number Diff line change
Expand Up @@ -35,8 +35,10 @@ def _node_module_library_impl(ctx):
for f in ctx.files.srcs
if f.path.endswith(".d.ts") and
# exclude eg. external/npm/node_modules/protobufjs/node_modules/@types/node/index.d.ts
# these would be duplicates of the typings provided directly in another dependency
len(f.path.split("/node_modules/")) < 3
# these would be duplicates of the typings provided directly in another dependency.
# also exclude all /node_modules/typescript/lib/lib.*.d.ts files as these are determined by
# the tsconfig "lib" attribute
len(f.path.split("/node_modules/")) < 3 and f.path.find("/node_modules/typescript/lib/lib.") == -1
])

# transitive_declarations are all .d.ts files in srcs plus those in direct & transitive dependencies
Expand Down

0 comments on commit 3d55b41

Please sign in to comment.