Guidance on shipping ts within node_modules

[johnnyreilly]

Hello!

I'm raising an issue because I'm seeking guidance on whether TypeScript is intended to be used to ship .ts files inside node_modules. I'm asking as I'm one of the maintainers of ts-loader and this issue was raised on that very topic. It seems to have worked in the past (without any particular intention or effort on the part of ts-loader) but broke with TypeScript 2.0.

@basarat took a look into it and ended up reaching this conclusion:

I have the lost the will to implement it and would not recommend anyone to write a package that ships .ts + .js and instead people should use outDir + .d.ts + .js.
Again for those interested reasons, the following bad things happen if you ship .ts + .js files

• If your package is used in nodejs by someone using outDir, your package messes up other peoples outDir option.
• As new stricter TypeScript compiler options are implemented they cannot be used by people that use your package unless you update your package to compile with those options as well.

@basarat also suggested

We can provide an additional error message if path contains node_modules that says:
Since the file is in node_modules you should not need to recompile .ts files in node_modules and should contact the package author to request them to use --declaration --outDir.

I'm inclined to think this is a good idea and I was planning to implement this. Before I did so I wanted to see if this was in line with the way the TypeScript team intends the language to be used. I think this is probably encouraging good practice but I wondered if you could share a view on whether that's the case?

cc @jbrantly @HerringtonDarkholme

[mhegazy]

Can you elaborate on the issue, i could not get the details reading through the issue.

As for the recommendation, publish .js + .d.ts. this way your users do not have to compile your sources each time they need to refer to your library.

[Bobris]

As Typescript user... Bobril-build currently depends on ts in node_modules, because it uses Typescript AST to recognize calls to translation module - modifying AST to produce message ids, or detecting used resources, or usages of images for creating of sprites. Yes in theory all this stuff could be parsed out of JS, but question would it have same robustness, error reporting, speed? I am also thinking about advanced minification (Closure style or better) which is impossible just from JS code. But upgrade to TS 2.0 was not all roses we had to fix some incompatibilities in our TS code, so I feel pain too.

[mhegazy]

@Bobris I am afraid i do not understand the underlying issue. can you explain the problem or share a repro i can look at?

[Bobris]

Issue is that you fixing bugs in TypeScript compiler, making it stricter even without changing any TypeScript compiler options. This dir https://github.com/Bobris/Bobril/tree/7860bda9ac82520e2826ccbced38168eceb01cf6/package is compilable by TS 1.8.10 without errors, but not with TS 2.0.3.
So anybody using this in node_modules and upgrading TS will have hard time... That's why one of solutions is to not distribute TS, but just JS and D.TS which lowers probability of breakage.

[mhegazy]

I see. but if you publish sources, would not your have to compile with matching version of the tools you use, and with matching compiler options (e.g. --noImplicitAny, --noUnusedLocals, etc..) as well?

[Bobris]

But I think with TS 2.0 even just D.TS would not save me, because I made it null strict, so D.TS now contains | undefined which TS 1.8 does not understand I think, forcing users to upgrade to 2.0 anyway.
It is not big deal for us because I control used build tool (mentioned bobril-build). Just want to say, I don't want to loose this option (distribute JS and TS) completely because I don't control IDE (VS Code).
And because of bobril-build - it unify used compiler options (at least on minimum needed), but you can set it more strict - and that's why all my modules needs to be compilable under most strict compiler options available too.

[mhegazy]

Distributing .ts files is a supported scenario. We have users who use this with npm link at design time, and we do not intend to break this.

The OP was about recommendation. my recommendation is not to ship your .ts files to external users.

[johnnyreilly]

Thanks for the response @mhegazy.

Can you elaborate on the issue, i could not get the details reading through the issue.

That's a fair point - I have a feeling that there might be more than one scenario being discussed in the thread. Speaking for myself this is not a scenario I look to use at all and I'm not totally certain I understand what each poster has asked for - I'm just trying to grapple with what is reasonable to support and what is not.

Perhaps rather than me assuming wrongly I'll ask the posters on the original issue to pitch in with their expectations. So @ahelmel , @Tiedye, @g3r4n, @sumeet70 , @skysteve, @stevejhiggs could you pitch in with what you are hoping for here and lets see if we can identify if you're all after similar behaviours or if there is more than one issue going on here.

@basarat - could you also pitch in with what you were planning to support and then abandoned? Just so we can gather everything in one place? Thanks! 🌷

[stevejhiggs]

sure, both me and @skysteve are actually working on the same project so I'll detail our exact scenario:

• We have a large internal core library that provides 90% of our functionality and is just a collection of ts files.
• This library has multiple entry points (we reference more that just the entry point as defined within the package.json)
• On top of this library are a number of custom implementations that build on the functionality provided by the core.
• We want the end result to be consumed by the browser and to be as small as possible. We bundle via webpack + ts-loader.
• Because life likes to hand out problems we need the resulting script to run on ie8 so typescript is set to compile down to es3.

Now, we've actually worked around the problem for now as oddly our issues only occurred when "allowJs" was set to true and now we are typescript all the way through this is no longer needed. But with allowJs set to true we received errors along the lines of Typescript emitted no output for.. and I believe @skysteve provided a simple case to reproduce this issue in TypeStrong/ts-loader#278

[basarat]

There are actually three issues. Personas : Publisher is the person publishing an npm module and Consumer is the person using it 🌹

noEmit for isSourceFileFromExternalLibrary

These files are external (default host.isSourceFileFromExternalLibrary == true see TypeStrong/ts-loader#365 (comment) for breakdown). That is the issue discussed by @stevejhiggs here #12358 (comment) and all the people in this thread TypeStrong/ts-loader#278. We could fix it in ts-loader (adding own host.isSourceFileFromExternalLibrary) but I gave up and suggesting that we should improve the error message in ts-loader telling people not to shop .ts files in node_modules. Hence prompting this question by @johnnyreilly about best practice ❤️

Old code, New compiler

Publisher's .ts files must conform to all the strict compiler options Consumer wants. This is fairly clear I believe. If you ship .ts files its just the way it has to be. This is the issue being discussed by
@Bobris here #12358 (comment)

outDir

I believe it messes the outDir for Consumer. This is purely psychic debugging but I think its a safe assumption (@tomitrescak is typestyle/typestyle#34 fixed after I stopped publishing .ts files right?)

[johnnyreilly]

Thanks everyone that has commented here; that's really helpful 🌹

@mhegazy, given you've said:

As for the recommendation, publish .js + .d.ts. this way your users do not have to compile your sources each time they need to refer to your library.

and also given the other scenarios outlined in this issue and @basarat's specific suggestion that, in the case where there is no emit, we execute the following code:

    if (outputText === null || outputText === undefined) {
        const additionalGuidance = filePath.indexOf('node_modules') !== -1 
        ? "You should not need to recompile .ts files in node_modules and should contact the package author to request them to use --declaration --outDir. More https://github.com/TypeStrong/ts-loader/issues/278"
        : "";
        throw new Error(`Typescript emitted no output for ${filePath}.${additionalGuidance}`);
    }

Which will prompt the user that having node_modules packages which contain raw .ts files is a bad thing. Does that seem reasonable?

My gut feeling is that it is but I wanted to check with the TypeScript team before adding something that might propogate bad practice. (I don't think this will)

[mhegazy]

sounds reasonable. I should note that we have issue #11946 tracking emitting node_modules files if they are listed explicitly in the include/files list in the config file. this allows users who realy want to include the node_modules*.ts into their compilation.

[johnnyreilly]

Thanks @mhegazy - I'll plan to make the comment link back to this issue as this has more context.