prefer-export-from: Fix TypeScript compatibility

[nrgnrg]

closes #1727

When applying a fix checks the exportKind and adds type as a prefix to the specifier if it's a type export.

[nrgnrg]

@fisker

[fisker]

I'm sorry to say this is not correct solution, those "imported" can also variables.

I don't use ts myself, but afaik, exportKind can be on specifier and declaration(correct me if I'm wrong). We need get all imported exported kind. And make sure we don't reuse the wrong export.

Example:

import { foo } from "foo";
export {foo};
export type  {bar } from "foo";

The export can't be reused, need insert another export.

[fisker]

Normally I don't dig deep for the ts parser, we can also simply skip the fix if you see any importKind or exportKind is "type" if you find hard to fix. So the fix won't break your code until someone make a better fix.

[nrgnrg]

I'm not sure I follow exactly.

Given your example the correct fix would be either:

export { foo, type bar } from "foo";

or

export { foo } from "foo";
export type { bar } from "foo";

Which this fix correctly substitutes, I've run this change with --fix on our largish codebase at work and it's made the correct substitution every time.

[fisker]

What's the result for the code I posted? (example updated)

[nrgnrg]

Ah ok I gotcha, it's reusing the export type in this instance and converting foo to a type export.

[nrgnrg]

This is perhaps not the most perfect solution because we could be smarter about it, but by just ignoring export declarations which are type declarations as far as i can see we cannot end up in a scenario where a value is converted to a type.

[nrgnrg]

Your example now auto fix's to this:

export type { bar } from "foo";

export { foo } from "foo";

[sindresorhus]

This will need a test.

[nrgnrg]

I've corrected the test failures and added a test for the change.

[nrgnrg]

I think I've addressed you comments, I now see the following output for the given inputs (before and after this change).

Previously: value exports converted to type exports breaking code, or vice versa
Now: value exports preserved, type export style preserved (inline or multiple statements)

// input
import { foo } from "foo";
export { foo };
export type { bar } from "foo";

// Existing Transform:
export type { bar, foo } from "foo";

// Updated Transform:
export type { bar } from "foo";
export { foo } from "foo";

// input
import { foo } from "foo";
export { foo };
export { type bar } from "foo";

// Existing Transform:
export { type bar, foo } from "foo";

// Updated Transform:
export { type bar, foo } from "foo";

// input
import foo, { bar } from "foo";
export type { bar };
export default foo;

// Existing Transform:
export type { bar, default } from "foo";

// Updated Transform:
export type { bar } from "foo";
export { default } from "foo";

// input
import { foo } from 'foo';
export { foo };
export type { bar } from "foo";
export { baz } from "foo";

// Existing Transform:
export type { bar, foo } from "foo";
export { baz } from "foo";

// Updated Transform:
export type { bar } from "foo";
export { baz, foo } from "foo";

// input
import { foo } from 'foo';
export { foo };
export { type bar } from "foo";
export { baz } from "foo";

// Existing Transform:
export { type bar, foo } from "foo";
export { baz } from "foo";

// Updated Transform:
export { type bar, foo } from "foo";
export { baz } from "foo";

[fisker]

Always add tests when you do logic change.

[nrgnrg]

@fisker the additional test cases have been added now

[fisker]

I'm working to make some improvements.

[fisker]

@nrgnrg Can you take a look? Main improvements are in 03ba7c6.

1. Reuse type export declaration
2. Insert a type export instead of a value export

I don't use typescript myself, but I assume

export type {foo} from 'foo';

is preferred over

export {type foo} from "foo";

?

Are those changes good to you?

[fisker]

import type {foo} from  'foo';
export type bar = foo;

This seems not handled by our rule, if you are interested, maybe you can try to fix this in a separate PR?

[nrgnrg]

Your changes look good to me 👍

The inline type import/export syntax (export { type foo } from "foo";) is a more recent addition to the language which was added to avoid several import/export statements. Personally I find it more concise but the export type { foo } from 'foo' syntax would support older versions of TS.

The responsibility of enforcing one style over the other does not feel like the responsibility of this rule, there is a tracking issue in typescript-eslint to enforce consistent usage of the inline syntax by making it a part of import/no-duplicates so I think as long as this rule plays nicely with changes made there to enforce the inline syntax this is probably the correct way to go here in order to support older typescript versions.

[fisker]

Thanks for the information, I guess I was wrong, can you change it to prefer value export first? It should be simple.

[nrgnrg]

can you change it to prefer value export first? It should be simple.

Inline type syntax? yeah I'll try and find the time this week to update that.

Thanks for your work helping on this

[nrgnrg]

@fisker let me know how this looks

[fisker]

Actually, I mean 3020b59, if {type foo} is preferred, we should insert it in a value export, even type export exists.

@sindresorhus What's your opinion on this

import {type foo} from 'foo';
export {type foo}
export type {bar} from 'foo';
export {baz} from 'foo';

FIX 1:

export type {bar, foo} from 'foo'; // Reuse this type export.
export {baz} from 'foo';

FIX 2:

export type {bar} from 'foo';
export {baz, type foo} from 'foo'; // Reuse this value export.

[sindresorhus]

I would go with fix 1. That's what most people would want, I think.

[fisker]

Okay, I'll revert.

[sindresorhus]

However, my preference would be:

export {baz, type foo, type bar} from 'foo';

Since it makes it possible to have a single statement.

I think there should be a TODO comment to switch to that when the TS version with it is more widespread.

[fisker]

@nrgnrg already mentioned this in #1728 (comment), that should be handled in a separate rule.

[fisker]

I'm fine with those changes, thanks for working on this, good job @nrgnrg .