Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

TypeScript 2.0: index.js not generated for source files in node_modules (since 2.0.0-dev.20160701) #9542

Closed
markvandenbrink opened this issue Jul 6, 2016 · 15 comments · Fixed by #9607
Assignees
Labels
Bug A bug in TypeScript Fixed A PR has been merged for this issue

Comments

@markvandenbrink
Copy link

markvandenbrink commented Jul 6, 2016

We use NPM packages for creating TypeScript libraries. This is great for TypeScript code which is used in multiple projects. We already use TypeScript 2.0 and since 20160701 there is a problem when compiling those sources. The index.ts file in those packages is compiled, but the resulting index.js is not emitted. Other dependencies in index.ts (as in the example below) are emitted as expected. Only index.js is missing. I think it should be possible to have NPM packages with just the TypeScript source files so you can use packages for shared code/libraries. This worked like a charm before 2.0.0-dev.20160701. Probably broken by #9459. Could be on purpose, but the behavior right now is not consistent. All sources are compiled and emitted, only the index.js files are missing which is strange and results in a headache ;-).

TypeScript Version: 2.0.0-dev.20160701 (2.0.0-dev.20160630 gives the right output).

Code to reproduce
Create the following structure:

node_modules\test-module\index.ts
node_modules\test-module\something.ts
test.ts

With the following code:

// node_modules\test-module\something.ts
export const SomeString: string = "Hello world";
// node_modules\test-module\index.ts
export { SomeString } from "./something";
// test.ts
import { SomeString} from "test-module";

console.log(SomeString); // Should output `Hello world`

Expected behavior:
When compiling test.ts the following files should be emitted:

node_modules\test-module\index.js
node_modules\test-module\something.js
test.js

Actual behavior:
The index.js file is missing and the output is:

node_modules\test-module\something.js
test.js

If 2.0.0-dev.20160630 is installed the output is as expected.

I understand that in some cases it is not desirable to compile the code in the node_modules folder. But in our case we emit the code to a outDir specified in tsconfig.json. As the complete source tree will be reflected to this outDir also the node_modules folder is created in the outDir. Just as expected. We can use something like webpack or Browserify to bundle the code. We think this should be possible. Just pure TypeScript code in the NPM package and no need to publish the compiled JS in there...

Maybe it is better to have a compiler option to disable compiling sources under node_modules so one can decide the desired behavior...

@weswigham
Copy link
Member

CC @billti

@mhegazy
Copy link
Contributor

mhegazy commented Jul 6, 2016

These files are considered "external" dependencies; while in your scenario, it is plausible that you want to build your own packages, it is fairly unexpected to change one of the dependencies that you do not own as you build. What if you have --noImplictAny and one of your dependencies does not? what if you are building with --module AMD where as your dependency is a UMD... This even gets more critical if you have --allowjs as now unexpected changes could just occur to installed packages, and that is very hard to detect and remedy. #6964 has more details about this issue.

For you scenario, I would suggest including either node_modules in your tsconfig.json, or only the node modules you want to build under node_modules. this way they will not be considered "external" and will be built like normal source files.

@mhegazy mhegazy added the Working as Intended The behavior described is the intended behavior; this is not a bug label Jul 6, 2016
@weswigham
Copy link
Member

weswigham commented Jul 7, 2016

@mhegazy Even if not compiling things in node_modules is working as intended, there's a bug here: Only the file found via package.json is exempted from compilation according to the report above - other files found under node_modules are not.

@mhegazy
Copy link
Contributor

mhegazy commented Jul 7, 2016

Yup, that is another one. We should do maintain the depth as we process references and imports.

@billti
Copy link
Member

billti commented Jul 7, 2016

One is a bug (only suppressing emit for the entry module) the other is by design (don't compile stuff found via searching node_modules). For the second, even if it's a root file, if at some point it is found via a node modules search, then I believe it is not emitted (as I did consider that, but considered it highly unlikely to be deliberate, and more likely a misconfigured "exclude" setting).

I can fix both pretty easy if desired to allow root files (i.e. specified Input source files) to be emitted even if later found by searching nose modules, as it sounds like this is a scenario customers are using. Thoughts?

@micah-williamson
Copy link

micah-williamson commented Jul 7, 2016

Im having the same issue as @markvandenbrink. I do want to include the ts source in my node_modules so the person using it does not need to manage additional definitions. Additionally, there isn't an easy way to compile typescript so there is a single point of entry without using system or amd.

If I have-

node_modules
    test
        foo.ts
        index.ts
index.ts

Where node_modules/test/foo.ts looks like-

export class Foo {}

and node_modules/test/index.ts looks like-

export {Foo} from './foo';

export class Bar {}

I want to be able to import Foo and Bar from index.ts like this-

import {Foo, Bar} from 'test';

@billti For myself, I don't expect to compile everything in node_modules or even everything from my own source. If the file is not imported then it would make sense that the compiler choses not to compile them. But I'm explicitly importing the index of those libraries so I would expect them to compile.

@markvandenbrink
Copy link
Author

markvandenbrink commented Jul 7, 2016

@billti It would be great if this behavior can be changed. It is really nice to use NPM packages to maintain TypeScript libraries (just like it is for typings). I think more people want to use it that way. We have a very large project and have to stick to 2.0.0-dev.20160630 for now. Missing all the bleeding edge stuff of last week ;-)

I fully understand the remarks of @mhegazy and the need for explicit inclusion of the desired sources in my tsconfig.json file. That makes sense and I could perfectly live with that. But this is not working as expected. I've included the sources in my tsconfig.json, but still no index.js. I've updated the test below to reproduce this.

As stated by @iamchairs my code is explicitly importing the node module source. But the confusing part in the current implementation is that the node module source is actually compiled as expected (even without explicit inclusion in tsconfig.json). Just the index.js is missing. But I understand this is a bug and no files should be generated for node modules, unless those sources are explicitly included in tsconfig.json.

So there is a bug in the current implementation: Only node modules which are included in tsconfig.json should be compiled and emitted. I've updated the test by adding a tsconfig.json to include the example node module in the compilation context and emit the output to a certain folder (in this case ./output). You can see the index.js file is still missing.

Tested with: 2.0.0-dev.20160707

Code to reproduce
Test code: 9542.zip

Create the following structure:

node_modules\test-module\index.ts
node_modules\test-module\something.ts
test.ts
tsconfig.json

With the following code:

// node_modules\test-module\something.ts
export const SomeString: string = "Hello world";
// node_modules\test-module\index.ts
export { SomeString } from "./something";
// test.ts
import { SomeString} from "test-module";

console.log(SomeString); // Should output `Hello world`
{
    "compilerOptions": {
        "target": "es5",
        "module": "commonjs",
        "outDir": "./output/"
    },
    "files": [
        "./test.ts",
        "./node_modules/test-module/index.ts",
        "./node_modules/test-module/something.ts"
    ]
}

Expected output:

output\node_modules\test-module\index.js
output\node_modules\test-module\something.js
output\test.js

Actual output:

output\node_modules\test-module\something.js
output\test.js

I hope this can be fixed. Thanks a lot for all the hard work!

@mhegazy
Copy link
Contributor

mhegazy commented Jul 7, 2016

@billti will you be sending a fix for this?

@billti
Copy link
Member

billti commented Jul 7, 2016

Yep. Tried to work on it already this morning but couldn't get a build working (see other issue I logged). Will try again once that is resolved.

@mhegazy mhegazy added the Bug A bug in TypeScript label Jul 7, 2016
@mhegazy mhegazy added this to the TypeScript 2.0.1 milestone Jul 7, 2016
@billti
Copy link
Member

billti commented Jul 11, 2016

I have a pull request out for this in #9542 against 'master' if you'd like to try this and see if it gives you what you're after. You can see how I've explicitly included/excluded some of the node_modules files in the test case at https://github.com/Microsoft/TypeScript/pull/9607/files#diff-ac1e756a59a142a0769a8d6bca92086aR7 .

@markvandenbrink
Copy link
Author

@billti this works very well! Just did some tests and there are no files generated for node modules if not explicitly specified in tsconfig.json. When I add the entry point of a node module (in my case index.ts) the node module gets compiled just as expected and all the right files are generated.

Do you think it will get merged in 2.0 or do we have to wait for 2.0.1?

@billti billti mentioned this issue Jul 11, 2016
@billti
Copy link
Member

billti commented Jul 11, 2016

I just created a pull request against release-2.0 also. I'll let @mhegazy make the call on what to merge when for which releases. Thanks for testing!

billti added a commit that referenced this issue Jul 11, 2016
Fix #9542 (allow files under node_modules to be included in the compilation).
@billti
Copy link
Member

billti commented Jul 11, 2016

This is in both master and the release-2.0 branch now. Thanks.

@mhegazy mhegazy added Fixed A PR has been merged for this issue and removed Working as Intended The behavior described is the intended behavior; this is not a bug labels Jul 11, 2016
@tgrecojs
Copy link

tgrecojs commented Aug 8, 2016

@billti is there any specific reason why filesGlob won't work in this example? I tried to see if I I would have an issue with swapping out files for filesGlob, and I noticed that it only compiled the one file.

Below you can see an example of my tsconfig.json file.

{
    "compilerOptions": {
        "target": "es5",
        "module": "commonjs",
        "outDir": "./output/"
    },
    "filesGlob": [
        "./test.ts",
        "./node_modules/test-module/**.ts"
    ]
}

I hope this isn't a silly question, but I figured it couldn't hurt to ask!

@billti
Copy link
Member

billti commented Aug 9, 2016

@thomasjosephgreco I believe filesGlob is an Atom thing. TypeScript doesn't understand that property. See http://www.typescriptlang.org/docs/handbook/tsconfig-json.html for the TypeScript fields for file globbing.

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
Bug A bug in TypeScript Fixed A PR has been merged for this issue
Projects
None yet
Development

Successfully merging a pull request may close this issue.

6 participants