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

ES2015 module issue with transpileOnly + decorators + interfaces #653

Open
RoystonS opened this issue Oct 16, 2017 · 23 comments

Comments

@RoystonS
Copy link

commented Oct 16, 2017

Hi there,

We have a very large Angular 2 codebase that we're attempting to move over to ts-loader; we're getting massive (4-5x) speed boosts compared to our existing awesome-typescript-loader setup, but sadly we're getting compilation issues when we turn on transpileOnly.

It only occurs when:

  • Building ES2015 modules AND
  • Using a class with a decorator on it AND
  • The class uses an imported interface AND
  • The imported interface comes from a module that also exports something concrete (e.g. a class or a const)

(webpack complains that it can't find the interface with a stack trace suggesting that it's processing harmony imports.)

I've created a repo with a standalone repro case in it: it's just tiny 2 files both of which are under 10 lines of code:

https://github.com/RoystonS/ts-loader-issue

(Is this related to what's discussed in issue #400?)

Thanks,
Royston.

@RoystonS

This comment has been minimized.

Copy link
Author

commented Oct 16, 2017

Ah, it's worse than I thought. Even after working around the warnings by moving interfaces to separate files, the actual generated code, is mashed, and the decorator references to even concrete types are broken.

Correct output code in __decorate call (transpileOnly off):

[__WEBPACK_IMPORTED_MODULE_1__ng_core__[\"HttpService\"]])]

Broken output code in __decorate call (transpileOnly on):

[typeof (_a = typeof __WEBPACK_IMPORTED_MODULE_1__ng_core__[\"HttpService\"] !== \"undefined\" && __WEBPACK_IMPORTED_MODULE_1__ng_core__[\"HttpService\"]) === \"function\" && _a || Object])]
@johnnyreilly

This comment has been minimized.

Copy link
Member

commented Oct 16, 2017

First of all: thanks for the detailed and minimal repro! That's always appreciated.

I'm somewhat intrigued as to what's causing your issue since I'm using similar approaches in this project without anything choking. I'm not sure why that would be.

TBH I'm somewhat time-poor right now and so I'm not likely to be able to look at this directly. That said, I'm happy to try and help you get to the bottom of it if I'm able to. I'd be tempted to advise doing the following:

  • fork ts-loader
  • put some extra logging in around the transpile emit
  • yarn link it into your project

See what, if anything that reveals...

@johnnyreilly

This comment has been minimized.

Copy link
Member

commented Oct 16, 2017

It may also be worth reading up on this issue: microsoft/TypeScript#18008 - I haven't read it thoroughly but it may be relevant. Also, does dropping emitDecoratorMetadata: true fix the issue?

@RoystonS

This comment has been minimized.

Copy link
Author

commented Oct 16, 2017

Thanks. I'll take a look at your 'hours' repro and see what's going on there.

Dropping emitDecoratorMetadata: true does stop the warnings appearing (as does removing any concrete export from the module with the interface in).

Our entire codebase only has about 3 instances of this in there, so I've managed to work around the interface warnings by moving the interfaces to their own files. But the generated code is still borked.

@RoystonS

This comment has been minimized.

Copy link
Author

commented Oct 16, 2017

I've had a look at your 'hours' repo, and out of the box it doesn't choke, as you say, but I can get it to choke with two small changes:

  1. Turn on emitDecoratorMetadata: true in the tsconfig.json file
  2. Reference an imported interface in a constructor of a decorated class. (That'll cause the compiler to emit metadata about the constructor parameters, which causes the problem.) For instance, add this constructor to src/components/layout/header.tsx:
constructor(props: RouteComponentProps<{}>) {
    super(props);
}

That then produces:

WARNING in ./src/components/layout/header.tsx
41:69-88 "export 'RouteComponentProps' was not found in 'react-router'
    at HarmonyImportSpecifierDependency._getErrors
    ...

This is a big issue for Angular 2+ which is reliant upon such metadata for its dependency injection. It looks like angular/cli is currently using a custom tsc wrapper for its TS compilation, and webpack for the rest of its build, but in our codebase we're using webpack all the way.

microsoft/TypeScript#18008 does indeed look relevant, especially the comment in microsoft/TypeScript#18008 (comment). Interestingly they suggest that the different generated code (a runtime check) that I see with transpileOnly: true is equivalent for interfaces, but it does appear to be incorrect for concrete classes, which is meaning that our Angular 2 components fail to detect their own dependencies.

@johnnyreilly

This comment has been minimized.

Copy link
Member

commented Oct 16, 2017

It could be worth reporting your experience on the TypeScript issue - they might be able to advise. I'd be interested if you've references to the relevant bit of the Angular cli...

@RoystonS

This comment has been minimized.

Copy link
Author

commented Oct 16, 2017

Yup, just logged a comment on the TS chain. It's proving a bit hard to find the angular wrapper, but there's a bit here:
https://github.com/angular/angular/tree/626555c01376f2daa8270db27c11606d48898ae6/packages/tsc-wrapped

@olee

This comment has been minimized.

Copy link

commented May 21, 2018

I am also experiencing this issue when I tried to use autobind-decorator on class methods for react components.
Those that use imported interfaces in their parameters start to throw these errors.
Are there any news on this?

@RoystonS

This comment has been minimized.

Copy link
Author

commented May 21, 2018

(I've not investigated further. Our team jumped ship to React as soon as FB changed the licence.)

@Strate

This comment has been minimized.

Copy link
Contributor

commented May 22, 2018

I use this plugin to remove warning. These warnings are useless, because TS already made a check for export exists. This plugin compatible with webpack 3 and 4

const ModuleDependencyWarning = require("webpack/lib/ModuleDependencyWarning")

module.exports = class IgnoreNotFoundExportPlugin {
    apply(compiler) {
        const messageRegExp = /export '.*'( \(reexported as '.*'\))? was not found in/
        function doneHook(stats) {
            stats.compilation.warnings = stats.compilation.warnings.filter(function(warn) {
                if (warn instanceof ModuleDependencyWarning && messageRegExp.test(warn.message)) {
                    return false
                }
                return true;
            })
        }
        if (compiler.hooks) {
            compiler.hooks.done.tap("IgnoreNotFoundExportPlugin", doneHook)
        } else {
            compiler.plugin("done", doneHook)
        }
    }
}
@johnnyreilly

This comment has been minimized.

Copy link
Member

commented Jun 28, 2018

The workaround solution suggested by @Strate is very helpful. I'd be open to baking this into ts-loader since it seems to have bitten a few people... Would anyone like to submit a PR?

@johnnyreilly

This comment has been minimized.

Copy link
Member

commented Jun 28, 2018

BTW I'd only apply the change when emitDecoratorMetadata: true - it doesn't seem to be necessary when this is false

@johnnyreilly

This comment has been minimized.

Copy link
Member

commented Jun 28, 2018

It looks like there's 3 places in the codebase where errors are registered to webpack. See the top 3 files in this search https://github.com/TypeStrong/ts-loader/search?utf8=✓&q=errors.push&type=

If in each call site we applied @Strate's filter then I imagine that would do the job. If someone wants to POC this and submit a PR we can work on this together. Don't worry about getting tests to pass - I can help with that.

@Roustalski

This comment has been minimized.

Copy link

commented Aug 29, 2018

I would also like to chime in here. We are using create-react-app with the react-scripts-ts that uses all webpack under the hood and Inverisfy JS for our IOC, and this is popping up all over the place. I'll try out the workaround.

@rolandjitsu

This comment has been minimized.

Copy link

commented Oct 8, 2018

@Roustalski I'm having the same problem, but setting emitDecoratorMetadata to false does not make the problem go away.

@RoystonS

This comment has been minimized.

Copy link
Author

commented Nov 18, 2018

(Note that I mentioned earlier (#653 (comment)), that at that time it wasn't simply a case of ignoring the warnings. The generated code was broken. I don't know whether anything has changed since, but merely suppressing/filtering the warnings might not be sufficient...)

@iamakulov

This comment has been minimized.

Copy link

commented Dec 4, 2018

Tuned @Strate’s plugin code to allow specifying what export warnings to ignore. I find this useful because such warnings have been helpful for me several times.

Usage:

// webpack.config.js
const IgnoreNotFoundExportPlugin = require('./IgnoreNotFoundExportPlugin.js');

module.exports = {
  plugins: [
    new IgnoreNotFoundExportPlugin(['MyInterface', 'MyAnotherInterface']),
    // Would ignore export warnings that look like
    //   export 'MyInterface' was not found in './file'
    // or
    //   export 'MyAnotherInterface' was not found in './file'
    // but not others
  ]
}

or

// webpack.config.js
const IgnoreNotFoundExportPlugin = require('./IgnoreNotFoundExportPlugin.js');

module.exports = {
  plugins: [
    new IgnoreNotFoundExportPlugin(),
    // Would ignore all “export not found” warnings
  ]
}

Plugin code:

See more
// IgnoreNotFoundExportPlugin.js
const ModuleDependencyWarning = require('webpack/lib/ModuleDependencyWarning');

// ↓ Based on https://github.com/sindresorhus/escape-string-regexp
const escapeStringForRegExp = string => string.replace(/[|\\{}()[\]^$+*?.]/g, '\\$&');

module.exports = class IgnoreNotFoundExportPlugin {
    constructor(exportsToIgnore) {
        this.exportsToIgnore = exportsToIgnore || [];
    }

    getMessageRegExp() {
        if (this.exportsToIgnore.length > 0) {
            const exportsPattern =
                '(' +
                this.exportsToIgnore.map(escapeStringForRegExp).join('|') +
                ')';

            return new RegExp(
                `export '${
                    this.exportsToIgnore
                }'( \\(reexported as '.*'\\))? was not found in`,
            );
        } else {
            return /export '.*'( \(reexported as '.*'\))? was not found in/;
        }
    }

    apply(compiler) {
        const messageRegExp = this.getMessageRegExp();

        const doneHook = stats => {
            stats.compilation.warnings = stats.compilation.warnings.filter(
                warn => {
                    if (
                        warn instanceof ModuleDependencyWarning &&
                        messageRegExp.test(warn.message)
                    ) {
                        return false;
                    }
                    return true;
                },
            );
        };

        if (compiler.hooks) {
            compiler.hooks.done.tap('IgnoreNotFoundExportPlugin', doneHook);
        } else {
            compiler.plugin('done', doneHook);
        }
    }
};
@aczekajski

This comment has been minimized.

Copy link

commented Jan 9, 2019

@iamakulov const from line 14 (const exportsPattern =) is never used, did you mean to use it in line 21 instead of this.exportsToIgnore?

@johnnyreilly johnnyreilly added the pinned label Jan 19, 2019

@iyinchao

This comment has been minimized.

Copy link

commented Feb 26, 2019

A modified version from @iamakulov 's code for my private project , supports passing regexp / string as ignore rules, to apply to file name or exported identifiers. Hope it can help a little:

Usage (in webpack config):

const IgnoreNotFoundPlugin = require('./IgnoreNotFoundExportPlugin.js');

plugins: [
  new IgnoreNotFoundPlugin({
      sourceFiles: [/\/files\/test$/, '../files/test2']   // Only ignore source file name matched with at least one of these regexp/string
      exportNames: [ 'IInterface' ] 
  })
]

// Or
plugins: [
  new IgnoreNotFoundPlugin()   // All warnings will be ignored
]

Plugin code:

Click to expand

const ModuleDependencyWarning = require('webpack/lib/ModuleDependencyWarning');

module.exports = class IgnoreNotFoundExportPlugin {
  constructor(option) {
    const op = {
      sourceFiles: [],
      exportNames: [],
      ...option
    };

    this.ignoredSourceFiles = op.sourceFiles;
    this.ignoredExportNames = op.exportNames;
  }

  apply(compiler) {
    const reg = /export '(.*)' \((imported|reexported) as '.*'\)? was not found in '(.*)'/;

    const doneHook = stats => {
      stats.compilation.warnings = stats.compilation.warnings.filter(warn => {
        if (!(warn instanceof ModuleDependencyWarning) || !warn.message) {
          return true;
        }

        const matchedResult = warn.message.match(reg);

        if (!matchedResult) {
          return true;
        }

        const [, exportName, , sourceFile] = matchedResult;

        const customRulesIgnore = {
          exportNames: false,
          sourceFiles: false
        };

        if (this.ignoredExportNames.length) {
          for (let i = 0; i < this.ignoredExportNames.length; i++) {
            const rule = this.ignoredExportNames[i];
            if (typeof rule === 'string' && rule === exportName) {
              customRulesIgnore.exportNames = true;
              break;
            } else if (rule instanceof RegExp && rule.test(exportName)) {
              customRulesIgnore.exportNames = true;
              break;
            }
          }
        } else {
          customRulesIgnore.exportNames = true;
        }

        if (this.ignoredSourceFiles.length) {
          for (let i = 0; i < this.ignoredSourceFiles.length; i++) {
            const rule = this.ignoredSourceFiles[i];
            if (typeof rule === 'string' && rule === sourceFile) {
              customRulesIgnore.sourceFiles = true;
              break;
            } else if (rule instanceof RegExp && rule.test(sourceFile)) {
              customRulesIgnore.sourceFiles = true;
              break;
            }
          }
        } else {
          customRulesIgnore.sourceFiles = true;
        }

        let ret = false;
        Object.keys(customRulesIgnore).forEach(key => {
          if (!customRulesIgnore[key]) {
            ret = true;
          }
        });

        return ret;
      });
    };

    if (compiler.hooks) {
      compiler.hooks.done.tap('IgnoreNotFoundExportPlugin', doneHook);
    } else {
      compiler.plugin('done', doneHook);
    }
  }
};

@markogresak

This comment has been minimized.

Copy link

commented Apr 11, 2019

I have created a IgnoreNotFoundExportPlugin, which is configurable by import path, for example, to match only TypeScript files. I believe this solution to be more scalable for future import warning, without having to fix the config for every new warning.

@weijian19391

This comment has been minimized.

Copy link

commented Apr 30, 2019

Tested the various solutions here, but somehow the following line always returns false:

warn instanceof ModuleDependencyWarning

Here's how the warn object looks like:

ModuleDependencyWarning: "export 'IEventState' was not found in './reducer' at Compilation.reportDependencyErrorsAndWarnings (~/node_modules/next/node_modules/webpack/lib/Compilation.js:1332:23) at Compilation.finish (~/node_modules/next/node_modules/webpack/lib/Compilation.js:1138:9) at hooks.make.callAsync.err (~/node_modules/next/node_modules/webpack/lib/Compiler.js:539:17) at _done (eval at create (~/node_modules/tapable/lib/HookCodeFactory.js:32:10), <anonymous>:9:1) at _err0 (eval at create (~/node_modules/tapable/lib/HookCodeFactory.js:32:10), <anonymous>:20:22) at Promise.all.then (~/node_modules/next/node_modules/webpack/lib/DynamicEntryPlugin.js:74:20) at process._tickCallback (internal/process/next_tick.js:68:7) at HarmonyExportImportedSpecifierDependency._getErrors (~/node_modules/next/node_modules/webpack/lib/dependencies/HarmonyExportImportedSpecifierDependency.js:383:11) at HarmonyExportImportedSpecifierDependency.getWarnings (~/node_modules/next/node_modules/webpack/lib/dependencies/HarmonyExportImportedSpecifierDependency.js:330:15) at Compilation.reportDependencyErrorsAndWarnings (~/node_modules/next/node_modules/webpack/lib/Compilation.js:1327:24) at Compilation.finish (~/node_modules/next/node_modules/webpack/lib/Compilation.js:1138:9) at hooks.make.callAsync.err (~/node_modules/next/node_modules/webpack/lib/Compiler.js:539:17) at _done (eval at create (~/node_modules/tapable/lib/HookCodeFactory.js:32:10), <anonymous>:9:1) at _err0 (eval at create (~/node_modules/tapable/lib/HookCodeFactory.js:32:10), <anonymous>:20:22) at Promise.all.then (~/node_modules/next/node_modules/webpack/lib/DynamicEntryPlugin.js:74:20) at process._tickCallback (internal/process/next_tick.js:68:7)

It clearly states that the warn is a ModuleDependencyWarning. Anyone has any idea what's going on?

For reference, i'm using the following code:

I use this plugin to remove warning. These warnings are useless, because TS already made a check for export exists. This plugin compatible with webpack 3 and 4

const ModuleDependencyWarning = require("webpack/lib/ModuleDependencyWarning")

module.exports = class IgnoreNotFoundExportPlugin {
    apply(compiler) {
        const messageRegExp = /export '.*'( \(reexported as '.*'\))? was not found in/
        function doneHook(stats) {
            stats.compilation.warnings = stats.compilation.warnings.filter(function(warn) {
                if (warn instanceof ModuleDependencyWarning && messageRegExp.test(warn.message)) {
                    return false
                }
                return true;
            })
        }
        if (compiler.hooks) {
            compiler.hooks.done.tap("IgnoreNotFoundExportPlugin", doneHook)
        } else {
            compiler.plugin("done", doneHook)
        }
    }
}
@moccaplusplus

This comment has been minimized.

Copy link

commented May 24, 2019

I have the same issue:

WARNING` in ./src/main/ts/util/component/RegionSelect.tsx 89:91-101
  "export 'FieldProps' was not found in '../../react/form/FormField'
   @ ./src/main/ts/component/deployment/instance/InstanceAddDefFieldset.tsx
   @ ./src/main/ts/component/deployment/add/DeploymentAddFieldset.ts
   @ ./src/main/ts/component/deployment/add/DeploymentAdd.tsx
   @ ./src/main/ts/component/deployment/add/DeploymentAddPage.tsx
   @ ./src/main/ts/AppRoutes.ts
   @ ./src/main/ts/component/IndexPage.tsx
   @ ./src/main/ts/App.ts
   @ `./src/main/ts/index.ts

with code:

import { DefaultProps } from '../../react/decorator/Props';
import { FieldProps, FieldState, FormField } from '../../react/form/FormField';

@DefaultProps({ value: '' })
export class RegionSelect extends FormField<string, FieldProps<string>, State> {

  @Inject
  private endpoint: DictionaryService;

  constructor(props: FieldProps<string>) {

But when I define type alias in the same file, I get rid off warning:

type Props = FieldProps<string>; // or interface Props extends FieldProps<string> {}

@DefaultProps({ value: '' })
export class RegionSelect extends FormField<string, Props, State> {

  @Inject
  private endpoint: DictionaryService;

  constructor(props: Props) {

So it seems like a leaky beahavour. What makes local interface different from the imported one that raises warning?

PS.
I use webpack 4 with ts-loader, and of course the issue occurs only with transpileOnly option enabled.

@spenceryue

This comment has been minimized.

Copy link

commented Jul 2, 2019

I had the same problem that @weijian19391 mentioned.

I used this condition instead:

warn.constructor.name === "ModuleDependencyWarning"

in place of

warn instanceof ModuleDependencyWarning
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
You can’t perform that action at this time.