Skip to content

Commit

Permalink
feat(extract)!: detect 'type-only' with inline imports/exports - BREA…
Browse files Browse the repository at this point in the history
…KING (#875)

## Description

Detect dependencies extracted from TypeScript as `'type-only'` also when
all of the inline imports/re-exports are qualified as `type`.

## Motivation and Context

[TypeScript 4.5 added inline type
imports](https://www.typescriptlang.org/docs/handbook/release-notes/typescript-4-5.html#type-modifiers-on-import-names).
`dependency-cruiser` has support for `'type-only'` in
`dependencyTypesNot`, but this only works when using type import
statements, and not when regular import statements have all their inline
imports qualified as `type`.

This can be fixed by moving the individual type imports to a separate
type import statement, but this is cumbersome for some of us since using
[ESLint's consistent-type-imports with `fixStyle:
'inline-type-imports'`](https://github.com/typescript-eslint/typescript-eslint/blob/main/packages/eslint-plugin/docs/rules/consistent-type-imports.md#fixstyle)
will prefer adding `type` inline even when all imports are `type`.

By extension this PR also implements the functionality for inline type
re-exports.

Closes #873.

## How Has This Been Tested?

- [x] green ci
- [x] additional unit tests

## Types of changes

- [ ] Bug fix (non-breaking change which fixes an issue)
- [ ] Documentation only change
- [ ] Refactor (non-breaking change which fixes an issue without
changing functionality)
- [ ] New feature (non-breaking change which adds functionality)
- [x] Breaking change (fix or feature that would cause existing
functionality to change)

## Checklist

- [x] 📖

  - My change doesn't require a documentation update, or ...
  - it _does_ and I have updated it

- [x] ⚖️
- The contribution will be subject to [The MIT
license](https://github.com/sverweij/dependency-cruiser/blob/main/LICENSE),
and I'm OK with that.
  - The contribution is my own original work.
- I am ok with the stuff in
[**CONTRIBUTING.md**](https://github.com/sverweij/dependency-cruiser/blob/main/.github/CONTRIBUTING.md).

---------

Co-authored-by: Sander Verweij <sverweij@users.noreply.github.com>
  • Loading branch information
alvaro-cuesta and sverweij committed Nov 26, 2023
1 parent 03d3d4b commit 19033e8
Show file tree
Hide file tree
Showing 2 changed files with 81 additions and 4 deletions.
25 changes: 21 additions & 4 deletions src/extract/ast-extractors/extract-typescript-deps.mjs
Expand Up @@ -8,16 +8,31 @@ const typescript = await tryImport(
meta.supportedTranspilers.typescript,
);

function isTypeOnly(pStatement) {
function isTypeOnlyImport(pStatement) {
return (
pStatement.importClause &&
(pStatement.importClause.isTypeOnly ||
(pStatement.importClause.namedBindings &&
pStatement.importClause.namedBindings.elements &&
pStatement.importClause.namedBindings.elements.every(
(pElement) => pElement.isTypeOnly,
)))
);
}

function isTypeOnlyExport(pStatement) {
return (
(pStatement.importClause && pStatement.importClause.isTypeOnly) ||
// for some reason the isTypeOnly indicator is on _statement_ level
// and not in exportClause as it is in the importClause ¯\_ (ツ)_/¯.
// Also in the case of the omission of an alias the exportClause
// is not there entirely. So regardless whether there is a
// pStatement.exportClause or not, we can directly test for the
// isTypeOnly attribute.
pStatement.isTypeOnly
pStatement.isTypeOnly ||
// named reexports are per-element though
(pStatement.exportClause &&
pStatement.exportClause.elements &&
pStatement.exportClause.elements.every((pElement) => pElement.isTypeOnly))
);
}

Expand Down Expand Up @@ -47,7 +62,9 @@ function extractImportsAndExports(pAST) {
module: pStatement.moduleSpecifier.text,
moduleSystem: "es6",
exoticallyRequired: false,
...(isTypeOnly(pStatement) ? { dependencyTypes: ["type-only"] } : {}),
...(isTypeOnlyImport(pStatement) || isTypeOnlyExport(pStatement)
? { dependencyTypes: ["type-only"] }
: {}),
}));
}

Expand Down
Expand Up @@ -117,6 +117,37 @@ describe("[U] ast-extractors/extract-typescript - type imports and exports", ()
);
});

it("extracts imports with inline type imports - only type imports", () => {
deepEqual(
extractTypescript(
"import { type slork, type klaatu } from './ts-typical';",
),
[
{
module: "./ts-typical",
moduleSystem: "es6",
dynamic: false,
exoticallyRequired: false,
dependencyTypes: ["type-only"],
},
],
);
});

it("extracts imports with inline type imports - mixing type and non-type", () => {
deepEqual(
extractTypescript("import { type slork, klaatu } from './ts-typical';"),
[
{
module: "./ts-typical",
moduleSystem: "es6",
dynamic: false,
exoticallyRequired: false,
},
],
);
});

it("extracts re-exports that explicitly state they only re-export a type", () => {
deepEqual(
extractTypescript("export type * as vehicles from './vehicles';"),
Expand All @@ -143,4 +174,33 @@ describe("[U] ast-extractors/extract-typescript - type imports and exports", ()
},
]);
});

it("extracts re-exports with inline type re-exports - only type re-exports", () => {
deepEqual(
extractTypescript("export { type foobar, type baz } from './vehicles';"),
[
{
module: "./vehicles",
moduleSystem: "es6",
dynamic: false,
exoticallyRequired: false,
dependencyTypes: ["type-only"],
},
],
);
});

it("extracts re-exports with inline type re-exports - mixing type and non-type", () => {
deepEqual(
extractTypescript("export { type foobar, baz } from './vehicles';"),
[
{
module: "./vehicles",
moduleSystem: "es6",
dynamic: false,
exoticallyRequired: false,
},
],
);
});
});

0 comments on commit 19033e8

Please sign in to comment.