Skip to content

When transpileOnly is enabled, ts-loader leaves in re-exports of types, causing webpack 4 to warn about export not being found #751

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

Closed
univerio opened this issue Mar 24, 2018 · 22 comments
Labels

Comments

@univerio
Copy link
Contributor

Expected Behaviour

For a project like this:

// bar.ts
export {IFoo, FOO} from "./foo";
export type IBar = {};
// foo.ts
export type IFoo = {};
export const FOO = 1;
// index.ts
import {FOO, IFoo, IBar} from "./bar";

declare function foo(i: number): IFoo;
declare function bar(): IBar;
foo(FOO);
bar();

Regardless of whether transpileOnly is enabled, the output for bar.ts should be (assuming module=ESNEXT):

export var FOO = 1;

Actual Behaviour

The actual output is:

export { IFoo } from "./foo";
export var FOO = 1;

which causes webpack to complain:

WARNING in ./src/bar.ts
1:0-34 "export 'IFoo' was not found in './foo'
 @ ./src/bar.ts
 @ ./src/index.ts

When transpileOnly is off, the module is correctly emitted. This may very well be a compiler bug, but I'm not sure how to use tsc to reproduce this.

Steps to Reproduce the Problem

git clone https://github.com/univerio/ts-loader-type-export-issue
cd ts-loader-type-export-issue
yarn
./node_modules/webpack-cli/bin/webpack.js

Location of a Minimal Repository that Demonstrates the Issue.

https://github.com/univerio/ts-loader-type-export-issue

@johnnyreilly
Copy link
Member

This may very well be a compiler bug, but I'm not sure how to use tsc to reproduce this.

I suspect this is a compiler issue yes. That said there may be good reason for it. I'd suggest asking a question on the Typescript project to confirm if this is expected behaviour or not. ts-loader is doing nothing special here; just hooking into the Typescript compiler API

@univerio
Copy link
Contributor Author

I think I understand it now. I think it has to do with the fact that transpileOnly uses ts.transpileModule, which is essentially equivalent to turning on isolatedModules. When compiling each module separately, the compiler does not have enough information to know whether an import is a type or not (see microsoft/TypeScript#15231), so while it eliminates imports that are never used in a value position (which works for imported types that are not exported), it cannot know whether an export is a type or not. So, fundamentally, ts.transpileModule is incompatible with type re-exports.

This issue has cropped up now because webpack 4 warns about these invalid exports, but the final bundle otherwise works fine. Perhaps there's an option in webpack to silence these warnings?

@johnnyreilly
Copy link
Member

Thanks for investigating and reporting back - super helpful! It might be worth inquiring against the webpack repo for such an option and linking back to this. I'm not aware of an option myself but that doesn't mean it doesn't exist!

@univerio
Copy link
Contributor Author

Found the option:

{
  stats: {
    warningsFilter: /export .* was not found in/,
  }
}

Perhaps we can add it to the readme?

@johnnyreilly
Copy link
Member

Sure - send a PR!

@FourwingsY
Copy link

Hi, I met this problem not as warning but as error.
So I cannot suppress it. Anybody has an idea?

@univerio
Copy link
Contributor Author

@FourwingsY Do you have a plugin that converts warnings into errors?

@FourwingsY
Copy link

@univerio Oh yes i got it. I've set config.module.strictExportPresence = true on webpack config. Thanks a lot!

@enoshixi
Copy link

This also works for me and gets the compiler to remove the type export all together:

import { IFoo } from "./foo";
export type IFoo = IFoo;

@bjeanes
Copy link

bjeanes commented Mar 6, 2019

That did not work for me, but the solution in babel/babel-loader#603 (comment) did:

import { T as a_T } from "./a";
export type T = a_T;

@bbil
Copy link

bbil commented Oct 17, 2019

I'm seeing this now in webpack 4.41.2, and the warning looks different, causing the recommended regex to not suppress the warning.

Compiled with warnings.

./src/middleware/sagas/appStartHelper.ts
Attempted import error: 'StartConfig' is not exported from '@instore/middleware/sagas/appStartHelper'

Not sure if this is a result of the version of webpack, typescript, or maybe some munging from create-react-app.

@bbil
Copy link

bbil commented Oct 17, 2019

^^ To avoid a DenverCoder9 situation, here's an update.

The example warningsFilter is correct, but if you are working with an ejected create-react-app project, then you need to add this to the build.js script, not to the webpack configuration!

For whatever reason, the warning I posted above is not what you should be basing your regex on. Since the actual warning vs what was being displayed are different strings.

        messages = formatWebpackMessages(
          stats.toJson({
            all: false,
            warnings: true,
            errors: true,
            warningsFilter: [
              /export .* was not found in/
            ]
          })
        );

@Jessidhia
Copy link

This is going to be an issue when using any compiler that has to work without having the full type information... like typescript itself in --isolatedModules mode. As long as there is no syntax to annotate type-only exports / reexports this is going to persist as a problem.

@johnnyreilly johnnyreilly reopened this Oct 31, 2019
@johnnyreilly
Copy link
Member

johnnyreilly commented Oct 31, 2019

I've reopened this issue following @MLoughry comments on the webpack 5 beta. It seems this issue may present with webpack 5 as maybe an error whereas with webpack 4 it's a warning.

See webpack/webpack#9802 (comment)

If people could detail:

  1. What they're experiencing
  2. If there are any remedies

That would be helpful. It's not clear to me that the problems being experienced can be solved by ts-loader. Any "fix" would likely need to be implemented within the TypeScript APIs or handled by webpack configuration.

I'm reopening this ticket for visibility and in the hope that people can use this as a place to discuss the issues. If people could comment that would be amazing.

As it is, ts-loader is a bridge between webpack and TypeScript. It can't (I think) solve the problem of differing needs / aims which this change in approach by webpack represents. But hopefully it can encourage discussion and collaboration between the two 🤗

cc @andrewbranch @sokra

@johnnyreilly
Copy link
Member

See microsoft/TypeScript#34750 also as suggested by @andrewbranch

@Bnaya
Copy link

Bnaya commented Nov 2, 2019

it's possible to re-export types:

export type ExternalArgs = import("./internal/interfaces").ExternalArgsApi;
export type CreateObjectBufferOptions = import("./internal/api").CreateObjectBufferOptions;

@eltonchan
Copy link

config.module.strictExportPresence = true

should be set config.module.strictExportPresence = false

@johnnyreilly
Copy link
Member

johnnyreilly commented Jan 4, 2020

With microsoft/TypeScript#35200 (comment) merged I wonder if we should consider changing the default tsconfig compiler flag to:

--importsNotUsedAsValue "preserve"

For webpack 5 given that:

preserve keeps imports used only for types in the emit as a side-effect import

I think this might resolve the issue reported here so most users just don't bump into it.

We'd look to do this as a breaking changes version if we did.

A little background: as policy we deliberately don't change the default compiler flags often so that ts-loader can generally be treated as a drop in replacement for tsc. But this would seem to be a case which is very webpack specific and consequently appropriate. See:

const compilerOptions = Object.assign({}, configParseResult.options, {
skipLibCheck: true,
suppressOutputPathCheck: true // This is why: https://github.com/Microsoft/TypeScript/issues/7363
} as typescript.CompilerOptions);

I'd be interested in your thoughts @andrewbranch ?

@andrewbranch
Copy link
Contributor

I don’t think you need to change the default. You can still do export type { T } from './m' to solve the original issue here regardless of the emit setting. We will probably recommend preserve or error for new projects going forward, but I don’t see a strong reason to override the default in ts-loader, unless I’m missing something.

@johnnyreilly
Copy link
Member

Yeah that seems reasonable. I may put some docs in ts-loader to explicitly recommend using one of those too. Happy New Decade by the way! 🥰🤗

@stale
Copy link

stale bot commented Mar 6, 2020

This issue has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs. Thank you for your contributions.

@stale stale bot added the wontfix label Mar 6, 2020
@stale
Copy link

stale bot commented Mar 13, 2020

Closing as stale. Please reopen if you'd like to work on this further.

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

No branches or pull requests

10 participants