microsoft/TypeScript

Do not compile ts code of external module

vvakame opened this issue · 10 comments

#2338 implements npm module lookup (very cool!)

I have a request.
I want to tsc lookup .d.ts first, not .ts.

in above case.
code dependencies are resolved to node_modules/commandpost/index.d.ts -> node_modules/commandpost/lib/index.ts when my code exec import * as commandpost from "commandpost";
I want to tsc resolve node_modules/commandpost/index.d.ts -> node_modules/commandpost/lib/index.d.ts, not .ts.

$ tree node_modules/commandpost
node_modules/commandpost
├── LICENSE.txt
├── README.md
├── dtsm.json
├── index.d.ts
├── lib
│   ├── argument.d.ts
│   ├── argument.js
│   ├── argument.js.map
│   ├── argument.ts
│   ├── command.d.ts
│   ├── command.js
│   ├── command.js.map
│   ├── command.ts
│   ├── index.d.ts
│   ├── index.js
│   ├── index.js.map
│   ├── index.ts
│   ├── option.d.ts
│   ├── option.js
│   ├── option.js.map
│   ├── option.ts
│   ├── utils.d.ts
│   ├── utils.js
│   ├── utils.js.map
│   └── utils.ts
├── package.json
└── tsconfig.json

external ts code can't compile everywhere.
for example.

$ cat node_modules/commandpost/lib/index.ts
/// <reference path="../node_modules/typescript/lib/lib.es6.d.ts" />

"use strict";

try {
... omitted ...

but node_modules/commandpost/node_modules/typescript/lib/lib.es6.d.ts is not exists (typescript is devDependencies

Does TypeScript team has best practise about project structure for npm module?

commandpost

other case.

when npm package has ambient external module and proper external module both.
tsc lookup proper external first. I think this is unexpected breaking change.

for example.
actual (tsc 1.6.0-beta

$ find . -type f
./index.ts
./node_modules/foo/index.d.ts
./node_modules/foo/lib/bar.d.ts
./node_modules/foo/lib/buzz.ts

$ cat index.ts
import foo = require("foo");
foo.bar.bar();

$ cat node_modules/foo/index.d.ts
declare module 'foo' {
    export import bar = require("foo/lib/bar");
}

declare module 'foo/lib/bar' {
    function bar(): void;
}

$ cat node_modules/foo/lib/bar.d.ts
import * as buzz from "./buzz";
export function bar(): void;

$ cat node_modules/foo/lib/buzz.ts
export class Sample {
    private foo: NotInTopLevelContextButRequiredInFooPackageContext;
}

$ tsc --listFiles --noImplicitAny --target ES5 --module commonjs index.ts node_modules/foo/index.d.ts
node_modules/foo/lib/buzz.ts(2,15): error TS2304: Cannot find name 'NotInTopLevelContextButRequiredInFooPackageContext'.
/Users/vvakame/work/TypeScript/lib/lib.d.ts
node_modules/foo/lib/buzz.ts
node_modules/foo/lib/bar.d.ts
node_modules/foo/index.d.ts
index.ts

expected

$ tsc --listFiles --noImplicitAny --target ES5 --module commonjs index.ts node_modules/foo/index.d.ts
/Users/vvakame/work/TypeScript/lib/lib.d.ts
node_modules/foo/index.d.ts
index.ts

this should be in master and release 1.6, @vvakame can you give it a quick try.

@mhegazy It is not works fine still...

repro

$ git clone https://github.com/vvakame/review.js.git
$ cd review.js
$ npm install && grunt setup
$ tsc -v
message TS6029: Version 1.6.2
$ tsc -p lib
resources/grammar.d.ts
lib/js/exception.ts
typings/jquery/jquery.d.ts
typings/i18next/i18next.d.ts
lib/parser/walker.ts
lib/parser/parser.ts
lib/parser/analyzer.ts
lib/parser/validator.ts
lib/controller/configRaw.ts
lib/builder/textBuilder.ts
lib/builder/htmlBuilder.ts
node_modules/typescript/lib/lib.es6.d.ts
typings/node/node.d.ts
lib/typings/polyfill.d.ts
lib/typings/custom-colors.d.ts
lib/controller/config.ts
lib/parser/preprocessor.ts
lib/controller/controller.ts
lib/index.ts
lib/utils/utils.ts
lib/i18n/en.ts
lib/i18n/ja.ts
lib/i18n/i18n.ts
lib/model/compilerModel.ts
lib/builder/builder.ts
typings/update-notifier/update-notifier.d.ts
typings/js-yaml/js-yaml.d.ts
node_modules/commandpost/lib/utils.ts      <- not expected, use utils.d.ts instead of utils.ts
node_modules/commandpost/lib/option.ts        <- not expected, use option.d.ts instead of option.ts
node_modules/commandpost/lib/argument.ts   <- not expected, use argument.d.ts instead of argument.ts
node_modules/commandpost/lib/command.ts  <- not expected, use command.d.ts instead of command.ts
node_modules/commandpost/lib/index.ts         <- not expected, use index.d.ts instead of index.ts
node_modules/commandpost/index.d.ts     <- works fine!
lib/cli.ts
lib/utils/polyfill.ts
typings/assert/assert.d.ts
typings/mocha/mocha.d.ts
typings/bundle.d.ts

@vladima can you take a look.

@mhegazy this is a known issue with transitive imports from the 'external typings only' contexts that was not yet fixed

vvakame/commandpost@d637859
this is my workaround. it works fine 👍
I love tsc 1.6 😸

After going on this back and forth a few times, the current behavior seems to be the best. Users working on multiple modules at design time would like to get this behavior.
@vladima has a tool to verify a package with "recommended" setup, that interested package authors can run, instead of the compiler enforcing it.

@mhegazy cool, is that tool available somewhere?

@vladima can you share your tool?