Skip to content

Type import statement not erased when exported as default #40420

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
peaBerberian opened this issue Sep 7, 2020 · 12 comments · Fixed by #41904
Closed

Type import statement not erased when exported as default #40420

peaBerberian opened this issue Sep 7, 2020 · 12 comments · Fixed by #41904
Assignees
Labels
Bug A bug in TypeScript Fix Available A PR has been opened for this issue Has Repro This issue has compiler-backed repros: https://aka.ms/ts-repros

Comments

@peaBerberian
Copy link

TypeScript Version: 4.1.0-dev.20200907

Search Terms: default export type erased

(The keywords I found for that issue are too generic so it may have already been found. Sorry in advance if that is the case.)

Code

I found a strange behavior leading to an error when running the compiled file through node.js. In my [perhaps wrong] opinion, the issue is with how TypeScript compile a TypeScript file to a JS file.

I tried to reduce the issue to the bare minimum. The following code does not anything, but this is the simplification of a problem we encountered in a real bigger codebase.

Let's consider two files:

// exported.ts
type Foo = number;
export { Foo };
// main.ts
import { Foo } from "./exported";
export default Foo;

That's it. Note that there is no importance here that those files only contains types. The same thing happens when real "translate-able" JS code is also there.
The real important part seems to be the export default in main.ts.

Expected behavior:

The expected behavior here is that TypeScript notice that Foo is a type and completely erase it from the resulting JS file.

When compiling with tsc --module es2015 main.ts, I expect to have these two files outputed:

// exported.js

// either nothing, or something like:
export {};
// main.js

// either nothing, or something like:
export {};

And this is exactly what happens if Foo is exported as a named export in main.ts (nothing is written in those files in typescript <= 3.9.7, an empty export for typescript >= 4.0.2).

Actual behavior:

The output from typescript 4.0.2 onward (to 4.1.0-dev.20200907 for my last check) was instead those two files:

// exported.js
export {};
// main.js
import { Foo } from "./exported";

So Foo does have been erased from exported.js but it is kept as an import in main.js for no apparent reason. TypeScript still seems to realize that this is a type that should be erased in main.js as the export clause was removed.

When running the resulting main.js through node (I have to in some way communicate to node that those are ES modules before) I receive the following error:

SyntaxError: The requested module 'file:///exported.js' does not provide an export named 'Foo'

While writing this issue, I saw that an import type statement exists. If I do a import type { Foo } from "./exported"; in main.ts it does work as expected (main.js is now only an export {} statement).
I could stop here and consider this issue as already fixed then.

However, there are two things that makes me think this issue is still one even with that resolution:

  • that import type statement is defined as useful in conditions I did not meet: if you’ve hit issues under --isolatedModules, TypeScript’s transpileModule API, or Babel, this feature might be relevant.
  • older versions of TypeScript (e.g. v3.0.1) work without any problem on that code, and I didn't use any new feature.

Note that I wrote that this is a new issue in TypeScript v4.0.2 onwards but that is not exactly true. The issue exists at least since v3.8.3 (I just know it was not there in v3.0.1) but under another form: in older versions main.js still is the same way but exported.js appears as an empty file. The exact issue I had on my more complex code (strangely) appeared only since updating from v3.9.7 to v4.0.2, so I chose to specifically report this issue it for those new versions.

@RyanCavanaugh RyanCavanaugh added the Needs Investigation This issue needs a team member to investigate its status. label Sep 9, 2020
@janwidmer
Copy link

janwidmer commented Oct 1, 2020

We are experiencing a similiar behaviour:

When using an exported interface as Type for a method parameter in combination with a decorator, we get the described warning. The warnings started to popup with typescript version 4.0.2. When using typescript 3.9.4, the warnings did not occur for us.

The types.ts files contains only named exports

// types.ts

export interface IGEvent {
	data: {
		eventData: IGEventOptions;
	};
}

The index.ts imports the event interface and uses it within it's decorated method signature

// index.ts
import {Component, EventListener, GondelBaseComponent} from '@gondel/core';
import { IGEvent } from './types';

@Component('HelloWorld')
class HelloWorld extends GondelBaseComponent {

	@EventListener('gEvent')
	_handleEvent(event: IGEvent) {
		alert(event.data.eventData.helloWorld);
	}
}

The used library gondel is a component framework to start components on a page and handle their lifecylce.

The warning goes away when

  1. we remove the decorator
  2. we define event as any and cast it inside the method to the type IGEvent
  3. we import it with import type { IGEvent } from ...

Is this intended behaviour because / as result of the new typed import feature or did it maybe got broken in 4.x versions?
As described by @RyanCavanaugh, as soon as we use import type { ... } the warning goes away, but as written, we are also not meeting the conditions under where it should be useful.

You can reproduce the warnings with the zip file attached here : ts-bug.zip

  1. do npm i
  2. do npm run start
  3. see the warning WARNING in ./src/index.ts 17:88-95 "export 'IGEvent' was not found in './types' in the console

@RyanCavanaugh RyanCavanaugh added this to the TypeScript 4.2.0 milestone Oct 1, 2020
@typescript-bot typescript-bot added the Fix Available A PR has been opened for this issue label Dec 9, 2020
@andrewbranch andrewbranch added Bug A bug in TypeScript and removed Needs Investigation This issue needs a team member to investigate its status. labels Dec 9, 2020
@andrewbranch
Copy link
Member

Dear @typescript-bot, please do magic with this

// @module: es2015
// @showEmit

// @filename: exported.ts
type Foo = number;
export { Foo };

// @filename: index.ts
import { Foo } from "./exported";
export default Foo;

Workbench Repro

@typescript-bot typescript-bot added the Has Repro This issue has compiler-backed repros: https://aka.ms/ts-repros label Dec 9, 2020
@andrewbranch
Copy link
Member

@janwidmer what you’re describing is a completely different issue. You have emitDecoratorMetadata set, so TypeScript will want to use IGEvent as part of its metadata annotations if it’s a value. But because the webpack config you’re using is running the compiler as a single-file transpiler, it has no idea that IGEvent doesn’t exist. That code is inherently unsafe in single-file transpilation, which is exactly what import type is for. That said, we ought to be warning you about this in --isolatedModules (even though you don’t have that turned on—you should), and it looks like we’re missing that error. Given this info, I would

  1. Enable isolatedModules and make sure I’m right that there’s no error
  2. If that’s the case, search again for duplicates describing the lack of the error
  3. If there are no duplicates, file a new bug

@andrewbranch
Copy link
Member

@typescript-bot run repros

@typescript-bot
Copy link
Collaborator

Heya @andrewbranch, I've started to run the code sample repros for you. Here's the link to my best guess at the log.

@typescript-bot
Copy link
Collaborator

typescript-bot commented Dec 9, 2020

👋 Hi, I'm the Repro bot. I can help narrow down and track compiler bugs across releases! This comment reflects the current state of the repro in this issue running against the nightly TypeScript. If something changes, I will post a new comment.


Comment by @peaBerberian

👍 Compiled
Emit:

import { Foo } from "./exported";

Historical Information

Comment by @peaBerberian

Version Reproduction Outputs
3.7.5, 3.8.2, 3.9.2, 4.0.2, 4.1.2, Nightly

👍 Compiled
Emit:

import { Foo } from "./exported";

@janwidmer
Copy link

janwidmer commented Dec 10, 2020

@andrewbranch I added isolatedModules to the tsconfig, but still the same, when compiling with webpack, I get the warning:

WARNING in ./src/index.ts 37:61-68
"export 'IGEvent' was not found in './types'

When compiling with native ts

node_modules/.bin/tsc --project tsconfig.json --outDir dist

the warning does not pop up. We are also running on a quite old version of ts-loader in our webpack setup, so we will update that first and then check again how the behaviour is.

Just to make sure I understand you right, in your opinion (with activated isolatedModules option), native ts compiler should show a warning because of the isolated Modules and no import type present?

We will see how it behaves and might check for duplicates or file a bug after updating our ts-loader

@andrewbranch
Copy link
Member

Just to make sure I understand you right, in your opinion (with activated isolatedModules option), native ts compiler should show a warning because of the isolated Modules and no import type present?

Yeah, we don’t have warnings, but it should have an error. The warning you’re seeing from webpack has turned into a build-breaking error in Webpack 5 IIRC. The point of isolatedModules is to tell you ahead of time if a transpiler (like Babel or ts-loader in transpile mode) will not have enough information to know whether the code it produces at runtime is safe or not. That’s what’s happening here—ts-loader in transpile mode cannot possibly know whether IGEvent has a value meaning by only looking at index.ts, and so it stays in. Webpack is smart enough to detect this and tell you about it, and prior to v5, forgiving enough to fix up your runtime code. But at the core, what happened was you wrote something that could not be analyzed by ts-loader, and consequently ts-loader produced fundamentally broken output. isolatedModules is the way you tell the compiler that you’re going to use a transpiler, and so you want errors that prevent you from getting in this situation. So in this case, that error is missing.

@janwidmer
Copy link

@andrewbranch thanks for your detailed explanation, now its's crystal clear :)

@janwidmer
Copy link

@andrewbranch I guess, your fix #41904 will cover the issue not showing an error in native TS described by me and I don't need to create another bug? (wasn't totally sure if your fix is only for the original problem described in this ticket or for both)

@andrewbranch
Copy link
Member

@janwidmer no, #41904 only fixes the bug described by the OP.

@janwidmer
Copy link

janwidmer commented Jan 11, 2021

@andrewbranch I created a bugticket, #42281, as I haven't found any other existing ticket for it.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Bug A bug in TypeScript Fix Available A PR has been opened for this issue Has Repro This issue has compiler-backed repros: https://aka.ms/ts-repros
Projects
None yet
Development

Successfully merging a pull request may close this issue.

5 participants