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

Build fails if importing a component from node_modules library #278

Closed
ahelmel opened this issue Sep 7, 2016 · 48 comments
Closed

Build fails if importing a component from node_modules library #278

ahelmel opened this issue Sep 7, 2016 · 48 comments

Comments

@ahelmel
Copy link

ahelmel commented Sep 7, 2016

I added a custom library from node_modules (something like: node_modules/@my-lib) imported a typescript component from this library in my code with:
import { MyComponent } from '@my-lib/component/my-component';

The typescript loader fails with the following message:
Module build failed: Error: Typescript emitted no output for /Users/.../my-app/node_modules/@my-lib/component/my-component.ts at Object.loader (/Users/.../my-app/node_modules/ts-loader/index.js:456:15).

ts-loader version: 0.8.2
webpack version: 1.13.1
tsc version: 1.8.10

@Tiedye
Copy link

Tiedye commented Sep 8, 2016

I am having a similar issue, however, it only manifested when I removed a couple entries from my webpack configuration. I have no clue why. The module didn't even output to either of the entries I removed.

I'm using the new typescript/webpack though:
ts-loader: 0.8.2
webpack: 2.1.0-beta.21
tsc: 2.0.2

Update:
I updated the component in question so that it included precompiled js files. It then worked without an issue. I think the problem is the ts loader may not compile components in node_modules under certain circumstances even if node modules is not excluded in tsconfig.json.

@g3r4n
Copy link

g3r4n commented Sep 27, 2016

I find a solution : i passed from Typescript@2.0.3 to Typescript@1.8.10

@sumeet70
Copy link

sumeet70 commented Oct 3, 2016

Enguerrand,
Can you please explain how did you fix the error. I am not able to follow your comment related to type script versions.

@g3r4n
Copy link

g3r4n commented Oct 3, 2016

Execute the foowing command line :

npm install typescript@1.8.10 -g
npm install typescript@1.8.10 -D
npm install typescript@1.8.10

It will install typescript v1.8.10 as global package, dev dependencies package and local package.

@skysteve
Copy link

Also getting this with TS 2.0.3, and downgrading TS is not an option because of some of the things we're using.

Weirdly if I npm link the module in question it works fine, but again that's not really an ideal solution

@johnnyreilly
Copy link
Member

If someone is able to produce a repo which demonstrates the problem that could be helpful. From what people have said this repo should work with TS 1.8 but not work with TS 2.0. If that exists then it's a starting point for someone to investigate this.

@skysteve
Copy link

skysteve commented Oct 11, 2016

@johnnyreilly https://github.com/skysteve/ts-bug-example-2 is about the simplest example I could get. Let me know if you need any more details, but certainly on my mac on node v 6.6 I can replicate the issue.

Update

If I update the tsconfig.json file to remove the "allowJS" flag, the error seems to go away (branch https://github.com/skysteve/ts-bug-example-2/tree/disallow-js). Which is fine in this simple project, but in my real world project I need to mix JS and TS so that's not a proper fix

@johnnyreilly
Copy link
Member

Could you retest with ts-loader v0.9.3? We've just added allowJs support which may remedy the issue.

@stevejhiggs
Copy link

Problem persists with 0.9.3

@basarat
Copy link
Member

basarat commented Nov 9, 2016

typestyle seems incompatible with ts-loader.

ERROR in ./~/typestyle/csx/index.ts
Module build failed: Error: Typescript emitted no output for /Users/syedb/REPOS/tmp/browsertypestyle/node_modules/typestyle/csx/index.ts
    at Object.loader (/Users/syedb/REPOS/tmp/browsertypestyle/node_modules/ts-loader/dist/index.js:27:15)

Sample project:
browsertypestyle.zip

Use npm install and npm start. There we get the error.

Note : awesome-typescript-loader works with it, but it has its own issues that I'd rather not get into here. Mentioning it as its something that could to debugged to figure out root cause / fix 🌹

@johnnyreilly
Copy link
Member

johnnyreilly commented Nov 9, 2016

Thanks for the repro @basarat - I'd actually rather forgotten about this issue. I'll try and take a look when I get a moment. Hope you are having fun in the USA - what a time you picked for a holiday!

@basarat
Copy link
Member

basarat commented Nov 9, 2016

Created a PR with a simpler repro : #365 and note about what needs fixing 🌹

@basarat
Copy link
Member

basarat commented Nov 13, 2016

Advice to any library authors shipping .ts files as the main way for ts consumption: I did the same for typestyle as an experiment and the results were not satisfactory and will never go there again. Reasons covered here : https://github.com/typestyle/typestyle/releases/tag/v0.7.0 Suggest you use outDir + .js + .d.ts as well 🌹 ❤️

@johnnyreilly
Copy link
Member

Sounds painful!

@basarat
Copy link
Member

basarat commented Nov 14, 2016

@johnnyreilly the lesson was painful. But the setup isn't:

  • tsconfig
  • code in src/
  • outDir in lib/
  • Done 🌹

@johnnyreilly
Copy link
Member

🌷 So does the change you were talking about making to ts-loader still stand?

@basarat
Copy link
Member

basarat commented Nov 14, 2016

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.

Help wanted 🌹

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. More #278

@johnnyreilly
Copy link
Member

Thanks for the investigation and write up @basarat. What you've said completely makes sense. I like the idea of the error message you suggest. Would it be straightforward to implement?

@basarat
Copy link
Member

basarat commented Nov 14, 2016

Would it be straightforward to implement?

From

ts-loader/src/index.ts

Lines 34 to 36 in a438869

if (outputText === null || outputText === undefined) {
throw new Error(`Typescript emitted no output for ${filePath}`);
}

    if (outputText === null || outputText === undefined) {
        throw new Error(`Typescript emitted no output for ${filePath}`);
    }

To :

    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}`);
    }

Ofcourse this is untested code and devil will be in the details + adding a test similar to #365 🌹

@johnnyreilly
Copy link
Member

Nice!

@johnnyreilly
Copy link
Member

johnnyreilly commented Nov 14, 2016

FWIW it might be worth turning your findings into a blog post (if you're so minded). It's always an idea to spread the word when it comes to good practice and it seems like this is still new territory. Have you any idea if the TypeScript team have published any thoughts on this?

Can't stop thinking of this:

Somewhat overplayed I'm sure 😄

@HerringtonDarkholme
Copy link
Contributor

Sorry I still didn't get the point in the issue by just reading 😞

The problem is "if library author ship .ts in npm package, ts-loader cannot compile it in node_modules". Is this right?

@kpudlik
Copy link

kpudlik commented Jan 5, 2017

Unfortunately shipping js/d.ts files is not a solution for me. I have 2 private repositories:

  • project-web
  • project-common

Both repos are written in Typescript.
project-web depends on project-common so I decided to fetch project-common github repository by using npm and just do something like this in project-web:

import { CommonComponents } from 'project-common'

The problem is that I can't provide precompiled js/d.ts (mainly because of inlining sass files in ts files which is handled by webpack like: { styles: [require('sample.sass')] } so I need to bundle/compile two pure TS repos together to one bundle file (one comes from node_modules). Of course error occurs. The funny thing is that it occurs randomly.. Any solutions?

ERROR in ../~/project-common/index.ts
Module build failed: Error: Typescript emitted no output for /Users/kpudlik/project-web/node_modules/project-common/index.ts
    at Object.loader (/Users/kpudlik/project-web/node_modules/ts-loader/index.js:458:15)
 @ ./app/app.module.ts 3:26-54

@ricklove
Copy link

@basarat

What I have done so I don't need to include "/lib" in the import statements is to make an index.js file that exports the lib as the main entry point:

index.ts:

export * from './lib'

That works fine for me unless I need a specific file, but it's not a terrible inconvenience as is.

I haven't used typings for my own projects, but if I run into problems, I'll research that.

@basarat
Copy link
Member

basarat commented Jan 18, 2017

What I have done so I don't need to include "/lib" in the import statements is to make an index.js

Instead of that you can just change your main in package.json to point to a file in the lib folder 🌹

@basarat
Copy link
Member

basarat commented Jan 18, 2017

@kanitw
Copy link

kanitw commented Feb 17, 2017

@basarat

If your package is used in nodejs by someone using outDir, your package messes up other peoples outDir option.

For an NPM package, if we ship js files in tsconfig.json's outDir (e.g., dist/ or build/) and ship ts files in src, would this still mess up other people's outDir option? (I hope not.)

@basarat
Copy link
Member

basarat commented Feb 17, 2017

For an NPM package, if we ship js files in tsconfig.json's outDir (e.g., dist/ or build/) and ship ts files in src, would this still mess up other people's outDir option? (I hope not.)

No. Be sure to add typings path to your package.json 🌹

@andybarron
Copy link

andybarron commented May 18, 2017

I often use this kind of structure to arrange my source code:

proj/
  package.json
  node_modules/ <- external deps
  src/
    node_modules/ <- my code
      client/
      server/
      shared/

This lets me use e.g. import * as api from 'shared/api' instead of having to use relative paths. Unfortunately, this triggers a false positive for this error message, because ts-loader sees node_modules in the path name and assumes I'm compiling .ts files from my deps. :/

Any way to fix this, either via project config or an upstream patch? It's killing me, Smalls!

Edit: I should note that this pattern works fine with tsc, but not with webpack + ts-loader.

@johnnyreilly
Copy link
Member

johnnyreilly commented May 19, 2017

Hi @andybarron,

I guess you're being bitten by this: https://github.com/TypeStrong/ts-loader/blob/master/src/index.ts#L38

If you compile your own version of ts-loader without the check does it all work fine for you?

I should say that whilst I understand the desire to avoid relative path hell (boy does it eat into my time trying to work our the right paths) I'd be rather hesitant to use your approach. 😄

@andybarron
Copy link

I'll give that a shot and get back to you.

If there's a better solution, I've yet to hear it 😛

@johnnyreilly
Copy link
Member

Yeah - I feel your pain

@quantuminformation
Copy link

I'm trying to re-export a specific item from https://github.com/quantumjs/vanilla-typescript
but this loader fails to build with You should not need to recompile .ts files in node_modules.

s-panferov/awesome-typescript-loader#566 (comment)

@johnnyreilly
Copy link
Member

I get so many issues on this I'm wondering about enabling this behaviour behind an opt-in flag

@clement911
Copy link

I'm in the same boat here. We have a shared module that is imported by our main module and we would like to import the ts files from the shared module.

@johnnyreilly
Copy link
Member

johnnyreilly commented Apr 17, 2018

OK - if someone wants to put together a PR which enables this I'll happily take a look.

Ideal implementation:

  • New allowTsInNodeModules option for ts-loader that, if true does not perform the check that leads to throwing the You should not need to recompile .ts files in node_modules message here

  • Suffix the existing You should not need to recompile .ts files in node_modules... message with advice on how to opt into using this. Something suitably "warning-y" like But if you really want to do this; use the allowTsInNodeModules option

Don't worry about fixing the tests; I can help with that.

@amritk
Copy link

amritk commented Dec 20, 2018

I'm having this issue as well on Typescript 3.2.2 and ts-loader 5.3.1. Its strange though, it works in one of my projects and doesn't work in another.

@mreiche
Copy link

mreiche commented Feb 14, 2019

I'm facing this issue as well, but infrequently. Sometimes it works, sometimes not.

yarn start build
...
Done in 12.65s.

When try to build again:

yarn start build
...
ERROR in ./node_modules/t-systems-aurelia-components/src/value-converters/date-format-value-converter.ts
Module build failed (from ./node_modules/ts-loader/index.js):
Error: TypeScript emitted no output for xxx\node_modules\t-systems-aurelia-components\src\value-converters\date-format-value-converter.ts.
    at makeSourceMapAndFinish xxx\node_modules\ts-loader\dist\index.js:78:15)
    at successLoader (xxx\node_modules\ts-loader\dist\index.js:68:9)
    at Object.loader (xxx\node_modules\ts-loader\dist\index.js:22:12)
 @ ./src/main.ts
 @ ./node_modules/aurelia-webpack-plugin/runtime/empty-entry.js
 @ multi aurelia-webpack-plugin/runtime/empty-entry aurelia-webpack-plugin/runtime/pal-loader-entry aurelia-bootstrapper
...
error Command failed with exit code 2.

In webpack.config.js

{test: /\.ts$/, loader: "ts-loader", options: { allowTsInNodeModules: true }},

Edit: I fixed that by adding the sources of the package to my tsconfig.json:

"include": [
    "./src/**/*.ts",
    "./node_modules/t-systems-aurelia-components/src/**/*.ts"
  ],

@kmannislands
Copy link

kmannislands commented Mar 25, 2019

I believe I am running into this after restructuring a project with lerna + yarn workspaces.
First I get this error:

ERROR in ~/directory/node_modules/@monorepo/shared/src/security.ts
Module build failed (from ~/directory/node_modules/ts-loader/index.js):
Error: TypeScript emitted no output for ~/directory/node_modules/@monorepo/shared/src/security.ts. By default, ts-loader will not compile .ts files in node_modules.
You should not need to recompile .ts files there, but if you really want to, use the allowTsInNodeModules option.
See: https://github.com/Microsoft/TypeScript/issues/12358
    at makeSourceMapAndFinish (~/directory/node_modules/ts-loader/dist/index.js:78:15)
    at successLoader (~/directory/node_modules/ts-loader/dist/index.js:68:9)
    at Object.loader (~/directory/node_modules/ts-loader/dist/index.js:22:12)
 @ ~/directory/node_modules/@monorepo/shared/src/index.ts 76:9-30
 @ ./src/app/index.js
 @ multi ./src/app/index.js

and then when I add the specified flag, something like:

{
  // rest of config...
  module: {
    rules: [
      {
        test: /\.[jt]sx?$/,
        loader: 'ts-loader',
        options: {
          allowTsInNodeModules: true,
        },
        exclude: /node_modules((?!@monorepo).)*$/,
        include: /node_modules\/@monorepo\/|src\//,
      },
    ]
  }
}

It's right back to a message like:

ERROR in ~/directory/node_modules/@monorepo/shared/src/security.ts.
Module build failed (from ~/directory/node_modules/ts-loader/index.js):
Error: TypeScript emitted no output for ~/directory/node_modules/@monorepo/shared/src/security.ts.
    at makeSourceMapAndFinish (~/directory/node_modules/ts-loader/dist/index.js:78:15)
    at successLoader (~/directory/node_modules/ts-loader/dist/index.js:68:9)
    at Object.loader (~/directory/node_modules/ts-loader/dist/index.js:22:12)
 @ ~/directory/node_modules/@monorepo/shared/src/index.ts 79:9-36
 @ ./src/app/index.js
 @ multi ./src/app/index.js

Have to move on for now, but will see if I can make a minimal repro repo soon.

@ritmos
Copy link

ritmos commented Mar 28, 2019

Hello dear typescripters. The issue is not because of ts-loader, but because tsc defaults to exclude node_modules folder (https://www.typescriptlang.org/docs/handbook/tsconfig-json.html). They have a bad example showing how to exclude files with "node_modules" in it which leads to confusion. I have oppened an issue for that.

In any case, in order to allow ts-loader consume typescript source files directly from node_modules: edit tsconfig.json and add all the needed packages there. Like for example:

tsconfig.json

{
"compilerOptions": { },
"include": [
   "node_modules/@company", // all modules bellow from the company will be available
   "node_modules/@company/module", // specific module from the company
    "node_modules/module" // specific module
  ]
}

Another option that also worked in my case is to set ts-loader option transpileOnly to true

@kmannislands
Copy link

@ritmos yes, something similar ended up being the issue for me, not a bug with ts-loader after all

@andrewbranch
Copy link
Contributor

It looks like #773 fixed this—can this be closed, or are there any further concerns?

@johnnyreilly
Copy link
Member

I think you're right - closed.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests