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

import type should not have any effect on output code #41562

Closed
KeithHenry opened this issue Nov 17, 2020 · 7 comments
Closed

import type should not have any effect on output code #41562

KeithHenry opened this issue Nov 17, 2020 · 7 comments
Labels
Duplicate An existing issue was already created

Comments

@KeithHenry
Copy link

KeithHenry commented Nov 17, 2020

import type declarations should not appear in the JS output and should not have any effect on the JS output.

As of 4.0 adding import type to a file causes export {}; to be added to the output. This worked in 3.8

This breaks components that use ES modules syntax (because the rest of project also uses it) but will not be loaded as modules. Examples include service workers, web workers, shared workers and custom elements loaded as side effects.

This is a breaking change with no workaround possible in TS. External workarounds are possible.

The general case has already been raised as #41513, this issue is specifically for the subset where the files only contain import type declarations and no plain import statements.

The documentation for import type states:

import type only imports declarations to be used for type annotations and declarations. It always gets fully erased, so there’s no remnant of it at runtime.

This was the case in 3.8, it is not the case with 4.0

TypeScript Version: 4.0.5

Search Terms: import type, export {}

Code

Using import type in a file so that TS can check types:

import type { InterfaceType } from './MyLibrary';
const test: InterfaceType = {};

Note that this file is not a module and will not be loaded as a module.

Expected behavior:

JS output should not include any reference to the import type declaration:

const test = {};

Actual behavior:

JS output includes export {}; to force the output to be a module:

const test = {};
export {};

Related Issues:

@MartinJohns
Copy link
Contributor

Duplicate of #41513.

@KeithHenry
Copy link
Author

KeithHenry commented Nov 17, 2020

@MartinJohns It isn't because #41513 is the general case, i.e. don't always emit export {};

This issue is for the specific case where there are no import statements in the JS output. Even if you accept that #41513 is by design because export {}; is strictly spec compliant (and I don't) in this case that spec should not apply because the output JS is not a module.

Even if you accept that all JS files with import directives are modules and all modules must have an export, a TS file with only import type is a special case - it won't have import in the output JS and therefore doesn't need to force an export.

Or to put it another way, with import type I expect that I'm telling TS to use that type when checking the file but don't change the output. I don't expect import type on its own to force the output to be a module.

@MartinJohns
Copy link
Contributor

MartinJohns commented Nov 17, 2020

I thought #41513 explicitly mentioned "import type" as well. But my impression was still that it's intentional that any use of module-syntax (aka import/export) leads to the file being treated as a module.

This impression aligns with the comment by @andrewbranch in the PR that added type-imports: #35200 (comment)

When you write an import type declaration, it’s essentially the same as writing an import declaration that you never use in an emitting position, except that it’s enforced that you never use it in an emitting position. Now, since import type is not standard ES grammar, I think you could reasonably argue that we could implement whatever rules we wanted; that it would not be inherently incorrect to say that import type does not constitute a module. But ultimately, given how similar it is to writing a regular import of a type, I think maintaining consistency that all import declarations are module markers was the correct choice.

So if it's not a duplicate, it's at least working as intended.

@KeithHenry
Copy link
Author

KeithHenry commented Nov 17, 2020

@MartinJohns In that case this bug would become with the documentation.

I think you could reasonably argue that we could implement whatever rules we wanted;

I agree, but in 3.8's release the statement on import type was

import type only imports declarations to be used for type annotations and declarations. It always gets fully erased, so there’s no remnant of it at runtime. [my emphasis]

This needs to be amended in the 4.0 release notes and documentation to something like:

By using import type you are telling TS that you want to force the output to be a module, so the JS output will always include export {}; for consistency, even if there are no plain import statements.

It would also be really helpful to have some documentation, anywhere, on what we should do instead? Is the TS design intent that web workers/service workers/custom elements/etc should not be able to share type definitions with ES module code? I really don't want to have two versions of './MyLibrary.d.ts', one for import and another for /// <reference path="..." />. That used to be the case and it was a PitA.

Finally I disagree entirely with:

I think maintaining consistency that all import declarations are module markers was the correct choice.

If import and import type behave the same for consistency then what is the point of import type in the first place? In what context would I ever use import type if it always behaves consistently with plain import?

I'm using import type because I want the design/transpile time definition specifically without changing my JS output.

The assumption that all files that include import type are intended to be modules is wrong (as in not all users want this all the time). We can argue about whether that intent is more one way or the other, but what it does do it break all places where import type was used and not intended to output a module.

At the very least I think this needs to be configurable within TS. If you think the most common intent is that import type should force module output then: fine, make that the default, but we can't opt out.

Our only options are:

  • Don't use import type, or
  • Don't use TypeScript 4, or
  • Add additional build steps to strip this bug out so that the JS works again - frankly, if TS needs a post-process script to run in order for the output to be valid JS then that is a bug with TS.

@MartinJohns
Copy link
Contributor

MartinJohns commented Nov 17, 2020

I agree, but in 3.8's release the statement on import type was

import type only imports declarations to be used for type annotations and declarations. It always gets fully erased, so there’s no remnant of it at runtime. [my emphasis]

There is no remnant of import type at runtime.

This needs to be amended in the 4.0 release notes and documentation to something like:

I agree that fixing the bug should be mentioned in the breaking-changes section of the release notes.
But the behavior is mentioned on the documentation regarding modules: https://www.typescriptlang.org/docs/handbook/modules.html

In TypeScript, just as in ECMAScript 2015, any file containing a top-level import or export is considered a module.

You're having a top-level import, so the file is a module.

If import and import type behave the same for consistency then what is the point of import type in the first place? In what context would I ever use import type if it always behaves consistently with plain import?

The purpose of import type is described in the PR that I linked.

I'm using import type because I want the design/transpile time definition specifically without changing my JS output.

Then you're using it for something it was not designed or intended for.


Anyway, I'm going to unsubscribe. 🤷

@KeithHenry
Copy link
Author

There is no remnant of import type at runtime.

Apart from, you know, the export {}; that is a remnant of the import type at runtime and that wouldn't be there if I'd just used any.

But the behavior is mentioned on the documentation regarding modules: https://www.typescriptlang.org/docs/handbook/modules.html

Er, ok, let's look: https://www.typescriptlang.org/docs/handbook/modules.html#importing-types:

import type is always guaranteed to be removed from your JavaScript

Oh, ok then.

You're having a top-level import, so the file is a module.

There is no top level import in my JS file. Why does it have to be a module?

Then you're using it for something it was not designed or intended for.

I guess, but it's something that did work, now doesn't, and that there is no workaround for other than either opt out of TS or add something that fixes TS output afterwards. I don't get how this is not a problem with TS.

Anyway, I'm going to unsubscribe.

Fair enough, thanks for the help.

@KeithHenry
Copy link
Author

The fix for this is to replace all instances of import type X with import(...).X:

- import type { InterfaceType } from './MyLibrary';
+ type InterfaceType = import('./MyLibrary').InterfaceType;

I still think the side effect of import type is very obtuse to anyone not directly involved in writing it, and some kind of documentation explaining that import type will always force the JS output to be a module would be helpful.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Duplicate An existing issue was already created
Projects
None yet
Development

No branches or pull requests

3 participants