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

[Babel 7] [Typescript] Exported Interfaces from an import are kept in the generated JS code #8361

Open
ManRueda opened this Issue Jul 22, 2018 · 17 comments

Comments

Projects
None yet
@ManRueda
Copy link

ManRueda commented Jul 22, 2018

Bug Report

Current Behavior
If the Typescript code exports an interfaces that was imported from other module, that interface reference is kept in the Javascript code after transpilation.

Input Code

File A:

export interface ExportedInterface {
  value: string
}

File B:

import { ExportedInterface } from './file-a';
export { ExportedInterface }

export function test(): ExportedInterface {
  return { value: '1' };
}

Expected behavior/code
The result code should not keep reference of that interface because is a type.

export function test() {
  return {
    value: '1'
  };
}

Babel Configuration (.babelrc, package.json, cli command)

{
  presets: [
    ['@babel/preset-env', { modules: false }],
     '@babel/preset-typescript',
  ],
}

Environment

  • Babel version(s): v7.0.0-beta.54
  • Node/npm version: Node 10.5.0/npm 6.2.0
  • OS: OSX 10.13.5
  • Monorepo no
  • How you are using Babel: cli (but is the same with loader)

Possible Solution
Looks like the here is a comment in the code that could be related to this issue: https://github.com/babel/babel/blob/master/packages/babel-plugin-transform-typescript/src/index.js#L52

Additional context/Screenshots
The generated code currently is the following one:

File A:

// No error, the file is generated empty

File B:

import { IInterface } from './test-sub';
export { IInterface };
export function test() {
  return {
    value: '1'
  };
}
@babel-bot

This comment has been minimized.

Copy link
Collaborator

babel-bot commented Jul 22, 2018

Hey @ManRueda! We really appreciate you taking the time to report an issue. The collaborators
on this project attempt to help as many people as possible, but we're a limited number of volunteers,
so it's possible this won't be addressed swiftly.

If you need any help, or just have general Babel or JavaScript questions, we have a vibrant Slack
community that typically always has someone willing to help. You can sign-up here
for an invite.

@nicolo-ribaudo

This comment has been minimized.

Copy link
Member

nicolo-ribaudo commented Jul 23, 2018

What if it isn't an interface but a class? Since Babel works per-file and not per-project, it can't know.

@ManRueda

This comment has been minimized.

Copy link
Author

ManRueda commented Jul 23, 2018

Works with classes. But the example that i showed is a simplified version. In my real life problem that interface comes from an external package (npm) so i don't have access to change it to a class.

@nicolo-ribaudo

This comment has been minimized.

Copy link
Member

nicolo-ribaudo commented Jul 23, 2018

How does typescript handle it with the --isolatedModules flags? Ideally we should match that behavior.

@ManRueda

This comment has been minimized.

Copy link
Author

ManRueda commented Jul 23, 2018

Yeah that case is not supported by typescript, it gives this error: Cannot re-export a type when the '--isolatedModules' flag is provided..

What makes sense if the modules are treated in an isolated way.

I will have to find another way to structure and export my types.

@dfreeman

This comment has been minimized.

Copy link

dfreeman commented Jul 26, 2018

One option might be to do something like this so it's syntactically clear that ExportedInterface is only a type:

import * as FileA from './file-a';
export type ExportedInterface = FileA.ExportedInterface;

export function test(): ExportedInterface {
  return { value: '1' };
}
@tajo

This comment has been minimized.

Copy link

tajo commented Jul 31, 2018

I think I'm running into the same problem and @dfreeman's workaround works.

// fileA.ts
export interface ExportedInterface { }
// fileB.ts
import { ExportedInterface } from './fileA';

export { ExportedInterface };

Webpack complains that export 'ExportedInterface' was not found in './fileA. tsc is happy.

This makes both happy:

// fileB.ts
import * as FileA from './fileA';

export type ExportedInterface = FileA.ExportedInterface;

I assume the problem is Babel not stripping off imports / exports of ExportedInterface?

ZauberNerd added a commit to xing/hops that referenced this issue Sep 5, 2018

fix(typescript): install necessary plugins and presets for typescript
In order to use TypeScript with the new @babel/preset-typescript we need
to remove the flow-preset and add / change the decorators and
class-properties plugins.

With this change stripping types from .tsx? files mostly works.
The only issues that I have found are:
- babel/babel#7749
- babel/babel#8361

ZauberNerd added a commit to xing/hops that referenced this issue Sep 5, 2018

fix(typescript): revert to ts-loader and typescript instead of babel
The babel typescript preset does not work exactly as we want it to.

from: #613 (comment)
```
After discussing with @robin-drexler we decided to switch back to using
ts-loader and typescript as we did before in this preset.

Because with the Babel variant there are two kind of important features
missing (type imports and interface exports / re-exports) - at least
that we know of now.

Other reasons for reverting:

- The babel variant will probably always be running a bit behind in
  terms of new features
- chasing bugs between babel / typescript etc and not knowing exactly
  where they come from
- using ts-loader and typescript does work but there is no real benefit
  for us to do the switch to babel (at least for now)
```

- babel/babel#7749
- babel/babel#8361

ZauberNerd added a commit to xing/hops that referenced this issue Sep 5, 2018

fix(typescript): revert to ts-loader and typescript instead of babel
The babel typescript preset does not work exactly as we want it to.

from: #613 (comment)
```
After discussing with @robin-drexler we decided to switch back to using
ts-loader and typescript as we did before in this preset.

Because with the Babel variant there are two kind of important features
missing (type imports and interface exports / re-exports) - at least
that we know of now.

Other reasons for reverting:

- The babel variant will probably always be running a bit behind in
  terms of new features
- chasing bugs between babel / typescript etc and not knowing exactly
  where they come from
- using ts-loader and typescript does work but there is no real benefit
  for us to do the switch to babel (at least for now)
```

- babel/babel#7749
- babel/babel#8361

ZauberNerd added a commit to xing/hops that referenced this issue Sep 5, 2018

fix(typescript): revert to ts-loader and typescript instead of babel
The babel typescript preset does not work exactly as we want it to.

from: #613 (comment)
```
After discussing with @robin-drexler we decided to switch back to using
ts-loader and typescript as we did before in this preset.

Because with the Babel variant there are two kind of important features
missing (type imports and interface exports / re-exports) - at least
that we know of now.

Other reasons for reverting:

- The babel variant will probably always be running a bit behind in
  terms of new features
- chasing bugs between babel / typescript etc and not knowing exactly
  where they come from
- using ts-loader and typescript does work but there is no real benefit
  for us to do the switch to babel (at least for now)
```

- babel/babel#7749
- babel/babel#8361
@TroySchmidt

This comment has been minimized.

Copy link

TroySchmidt commented Oct 9, 2018

I can confirm that I have this issue using TypeScript and re-exporting interfaces as well.

@milesj

This comment has been minimized.

Copy link

milesj commented Oct 10, 2018

Importing types, and then re-exporting them from another file is not allowed in Babel, and will never work. Babel has no context in whether an import is a type or a value, and as such, treats them all as values.

You'll need to restructure your application to work around this.

@TroySchmidt

This comment has been minimized.

Copy link

TroySchmidt commented Oct 10, 2018

That is very unfortunate. The use case is that I create a folder to encapsulate the API calls from the React web app into a single folder. All the interfaces for interacting with the API are in that folder organized into various files. To make using this API in the React app easier, I think import and export those from an index.ts file. This means I can then do things like import { Todo } from 'api' and not care about what specific folder that came from. This is common practice with many libraries as well like @material-ui.
Material-UI library type re-export

@sebinsua

This comment has been minimized.

Copy link

sebinsua commented Oct 10, 2018

It is possible to re-export values though and in fact Material UI uses Babel to do so.

However, it's impossible to re-export types in Babel, since (as the GitHub issue shows) it will cause these types to end up within the JavaScript output and this shall cause runtime errors.

Babel transpiling TypeScript should be treated as if you are in --isolatedModules mode and in fact it is recommended that you switch this on within tsconfig.json in order to get relevant warnings.

I don't think it would be possible for Babel to determine whether an import is a type or a value (or both) without it having read the underlying types files, and presumably this would require something more complex than a syntax and transform plugin.

Of course, technically if there were a way to temporarily strip lines with pure TypeScript exports before running them through Babel that would work. I've done this on one of my own projects using a rollup plugin to automatically remove lines which I prefixed with a particular comment. That worked but it is a hack.

@milesj

This comment has been minimized.

Copy link

milesj commented Oct 10, 2018

@TroySchmidt What about this? export * from './types

@TroySchmidt

This comment has been minimized.

Copy link

TroySchmidt commented Oct 22, 2018

@milesj That does seem to work. So I will continue down that path. TypeScript classes could include additional functionality with methods on them. But these are used to define the interfaces of the JSON data that is loaded from the api. Using interface made sure that someone doesn't accidentally try to add class style functions and expect those to work.

@sorenhoyer

This comment has been minimized.

Copy link

sorenhoyer commented Nov 3, 2018

@tajo , like @milesj mentioned, instead of importing types and then re-exporting, simply do export { ExportedInterface } from './fileA';

@TroySchmidt importing types and then re-exporting shouldn't be an issue for the use case that you're describing.

Back to the use case in the topic.

In general I don't consider it a good practice to import a type/interface/something, use it and then (re-)export it from that same file. That sounds like some real spaghetti code.

Here's what I consider a better way of structuring your code, with emphasis on types.

If you have a folder per React component, you could have one file /components/MyComponent/types.ts within that folder for types, where you declare and export types (and nothing more) that are only relevant to that specific component.
If the components are simple, you may not want to dedicate an entire folder for it, and decide only to have it in a file /components/MyComponent.tsx. In that case you may have your "types" file in /components/types.ts and have all your different components' types within that single types file.
If the types are more generic, move them to another types file, one or more levels up, in the folder hierarchy, until on a level where all the relevant parts of your app is able to import and access it.
Lastly if types.ts turns to big and you need to abstract it even further you can turn the types.ts file into a folder and break it out into multiple files /types/SomeKindOfTypes.ts, which again may or may not be wired up in a /types/index.ts file (up to you).

This way of organizing your types is of course not tied to React, but can be used for all purposes:

root/src/utils/types.ts:

export interface InterfaceA {}
export interface InterfaceB {}

root/src/utils/index.ts:

export {
  InterfaceA,
  InterfaceB,
} from './types';

root/index.ts:

export {
  InterfaceA,
  InterfaceB,
} from './src/utils';
@klimashkin

This comment has been minimized.

Copy link

klimashkin commented Nov 13, 2018

@sorenhoyer but root/src/utils/index.ts might want to use interfaces from root/src/utils/types.ts before reexporting them. Otherwise why would they need to be declared under that folder?

@klimashkin

This comment has been minimized.

Copy link

klimashkin commented Nov 13, 2018

@dfreeman Can I somehow reexport enum like in your example?

import * as FileA from './file-a';
export type ExportedInterface = FileA.ExportedInterface;

export function test(): ExportedInterface {
  return { value: '1' };
}
@ColCh

This comment has been minimized.

Copy link

ColCh commented Dec 25, 2018

@tajo and other interested

for this source file with reexport

export { greet } from './Hello';

import * as HelloModule from './Hello';
export type GreetArgs = HelloModule.GreetArgs;
alert('REEXPORT');

such file is generated

webpackHotUpdate("main",{

/***/ "./src/Greet-reexport.ts":
/*!*******************************!*\
  !*** ./src/Greet-reexport.ts ***!
  \*******************************/
/*! exports provided: greet */
/***/ (function(module, __webpack_exports__, __webpack_require__) {

"use strict";
__webpack_require__.r(__webpack_exports__);
/* harmony import */ var _Hello__WEBPACK_IMPORTED_MODULE_0__ = __webpack_require__(/*! ./Hello */ "./src/Hello.ts");
/* harmony reexport (safe) */ __webpack_require__.d(__webpack_exports__, "greet", function() { return _Hello__WEBPACK_IMPORTED_MODULE_0__["greet"]; });


alert('REEXPORT');

/***/ })

})
//# sourceMappingURL=main.fa092eff35ed08dde4dd.hot-update.js.map

it seems that babel completely stripped it from the source

I event can't find GreetArgs in the JS bundle file nor imported module (import * as FooModule)

tested on newly created project with CRA

Yup, this is a hack, but we can refactor our code later to remove type reexports

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