type inference: ts_library behaves differently than tsc

[stephanwlee]

🐞 bug report

Affected Rule

ts_library

Is this a regression?

Uncertain if this is a regression.

Description

When using ts_library on very simple module that imports and make use of a method in ngrx, tsc fails to infer the return type. This contrasts with the vanilla tsc which successfully infers the type and builds the file.

🔬 Minimal Reproduction

https://github.com/stephanwlee/bazel-ngc-tsc-bad

Repro steps are detailed in the README.
Note that I am using Bazel 0.26.1.

🔥 Exception or Error

bad-ngc/BUILD.bazel:3:1: Compiling TypeScript (devmode) //:bug failed (Exit 1) tsc_wrapped failed: error executing command bazel-out/host/bin/external/npm/@bazel/typescript/bin/tsc_wrapped @@bazel-out/k8-fastbuild/bin/bug_es5_tsconfig.json

Use --sandbox_debug to see verbose messages from the sandbox
src/core.actions.ts:17:14 - error TS2742: The inferred type of 'meow' cannot be named without a reference to '../external/npm/node_modules/@ngrx/store/src/models'. This is likely not portable. A type annotation is necessary.

17 export const meow = createAction('[Core] wow');
                ~~~~
src/core.actions.ts:17:14 - error TS2742: The inferred type of 'meow' cannot be named without a reference to '../external/npm/node_modules/@ngrx/store/store'. This is likely not portable. A type annotation is necessary.

17 export const meow = createAction('[Core] wow');
                ~~~~
src/core.reducers.ts:52:14 - error TS2742: The inferred type of 'getActivePlugin' cannot be named without a reference to '../external/npm/node_modules/@ngrx/store/src/selector'. This is likely not portable. A type annotation is necessary.

52 export const getActivePlugin = createSelector(
                ~~~~~~~~~~~~~~~
src/core.reducers.ts:52:14 - error TS2742: The inferred type of 'getActivePlugin' cannot be named without a reference to '../external/npm/node_modules/@ngrx/store/store'. This is likely not portable. A type annotation is necessary.

52 export const getActivePlugin = createSelector(
                ~~~~~~~~~~~~~~~

Target //:bug failed to build
Use --verbose_failures to see the command lines of failed build steps.
INFO: Elapsed time: 11.643s, Critical Path: 3.50s
INFO: 0 processes.
FAILED: Build did NOT complete successfully

🌍 Your Environment

Operating System:

  
DISTRIB_DESCRIPTION="Debian GNU/Linux rodete (upgraded from: Ubuntu 14.04.5 LTS)"
  

Output of bazel version:

  
Build label: 0.26.1
Build time: Thu Jun 6 11:05:05 2019 (1559819105)
Build timestamp: 1559819105
Build timestamp as int: 1559819105
  

Rules version (SHA):

  
https://github.com/bazelbuild/rules_nodejs/releases/download/0.34.0/rules_nodejs-0.34.0.tar.gz
  

Anything else relevant?
I poked around tsc and tsc_wrapped and inspected which files are being traversed (printed modules that are being resolved by compilerHost). Nothing was out of ordinary for both versions -- i.e., tsc_wrapped also traversed through the typed dependency of @ngrx/store (store/typing.d.ts, store/index.d.ts, ... store/src/model.d.ts)

[a-ignatov-parc]

There are some issues reported in TS repo, so it's hard to say exactly where the problem is

• importHelpers with symlinks breaks type resolution microsoft/TypeScript#29221
• types in transitive dependencies causing errors after upgrade from 3.1: The inferred type of ... cannot be named without a reference to microsoft/TypeScript#30858

Unfortunately, we had the same problems in our code base. We test some possible fixes like adding paths to modules with there own typings that came not from @types to typeRoots, but we didn't succeed.

For now, we use workarounds with importing modules and adding types with typeof.

[pauldraper]

Encountering the same problem.

I'm guessing the issue is ts_library's "path": { "*": ... }.

Rather than using the traditional nodes_modules file layout, it uses path mappings to effectively use a different node_modules. And TypeScript gets confused by how to add imports that reference those files.

[pauldraper]

I also created repro here: https://github.com/pauldraper/ts-bazel-infer-import

The issue is that when generating import() for the declarations, tsc is (1) preferring a relative path and (2) erroring if the path includes node_modules (which it does; it's some ../external/npm/node_modules type path).

[pauldraper]

Additional info: Beginning in TS 3.2, tsc started checking the generated path for node_modules and failing if it found it.

Either removing that check, or making the inferred import prefer non-relative paths would fix this issue.

[alexeagle]

Note, one possible "escape hatch" is to use plain tsc rather than ts_library, see https://github.com/bazelbuild/rules_nodejs/blob/master/packages/typescript/docs/install.md#alternatives

[tony-scio]

@alexeagle I don't really want to eject from ts_library because then we'd lose some benefits like keeping the worker running.

To workaround it, sometimes have to specify function definitions twice. Like:

export const handleRequestError: (requestType: string, response: Response, auth?: AuthState) => void = (requestType: string, response: Response, auth?: AuthState) => {
  ...
}

Would definitely be nice to see this fixed so the duplication can be removed. Please let me know if there's any useful info I could provide.

[eordano]

Just stumbled upon this issue. It would be nice to see some fix. Maybe, a workaround would be to have the ability to set importHelpers in the tsconfig file from the ts_library rule.

[pope]

For now, we use workarounds with importing modules and adding types with typeof.

Might you be able to go into more details about what that looks like?

[a-ignatov-parc]

@pope sure!

For example, we have

import { welcomeUser, blockUser, unblockUser } from '../actions';

const mapDispatchToProps = {
    welcomeUser,
    unblockUser,
    blockUser,
};

Which fails to build with

/account-action-column.tsx:70:7 - error TS2742: The inferred type of 'mapDispatchToProps' cannot be named without a reference to '../../../../../../../../../external/npm/node_modules/redux'. This is likely not portable. A type annotation is necessary.

70 const mapDispatchToProps = {
         ~~~~~~~~~~~~~~~~~~

This problem can be fixed in two ways:

1. Explicitly define mapDispatchToProps type:

   import { welcomeUser, blockUser, unblockUser } from '../actions';
   
   const mapDispatchToProps: {
       welcomeUser: typeof welcomeUser;
       unblockUser: typeof unblockUser;
       blockUser: typeof blockUser;
   } = {
       welcomeUser,
       unblockUser,
       blockUser,
   };

2. Add explicit redux import:

   import _ from 'redux'; // Explicit import
   import { welcomeUser, blockUser, unblockUser } from '../actions';
   
   const mapDispatchToProps = {
       welcomeUser,
       unblockUser,
       blockUser,
   };

   Also, you will be require to add @npm//redux label to deps attribute.

[flolu]

Another abosulutely ugly way to prevent the error is by setting the type of the affected object to any.

Is there any progress on this issue?
I am encountering the problems when using @ngrx/store and @ngrx/effects

[flolu]

For example I get 3 errors from my auth.effects.ts:

The inferred type of 'login$' cannot be named without a reference to '../../../../../external/npm/node_modules/@ngrx/effects/effects'
The inferred type of 'login$' cannot be named without a reference to '../../../../../external/npm/node_modules/rxjs'
The inferred type of 'login$' cannot be named without a reference to '../../../../../external/npm/node_modules/@ngrx/store/src/models'

and importing those 3 modules into auth.effects.ts fixes the errors:

import * as __ngrxEffectsTypes from '@ngrx/effects/src/models';
import * as __ngrxStoreTypes from '@ngrx/store/src/models';
import * as __rxjsTypes from 'rxjs';

Is it possible to import those modules once, e.g. in app.module.ts? I would be ok with this workaround when I wouldn't have to import those modules in every file I use ngrx

[Toxicable]

@flolu Those errors there you can solve by adding a explicity (rather than implicity) type to whatever is throwing the error.
eg: before

getPerson() { //return type is inferred to be Observable<Person>
  return this.person$; // where this.person is a Observable<Person>
}

replace with

getPerson(): Observable<Person> {
  return this.person$;
}

This is an actual a tsc error, but it's not a bug. Since with ts_library you usually compile your app in smaller chunks, so in order for another target to use the types from another they need to have those references explicity referenced in certain palces.
If you compiled all of your code in a single pass this wouldn't be an issue.