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

Issue: @import type not working #964

Closed
louwers opened this issue Nov 11, 2024 · 5 comments
Closed

Issue: @import type not working #964

louwers opened this issue Nov 11, 2024 · 5 comments
Labels
enhancement this will make dependency-cruiser sweeter

Comments

@louwers
Copy link

louwers commented Nov 11, 2024

This creates a dependency:

import type * as t from "some-package";

This does not:

/** @import * as t from "some-package" */

Expected Behavior

I expect a (type) dependency to be created by the second form.

This syntax was introduced in TypeScript 5.5. https://devblogs.microsoft.com/typescript/announcing-typescript-5-5/#the-jsdoc-import-tag

Current Behavior

No dependency is created (even when tsPreCompilationDeps is true).

Possible Solution

Not sure. Could be an upstream (TypeScript) bug?

Steps to Reproduce (for bugs)

index.js

/** @import * as t from "./types.js" */

/** @returns {t.Boo} **/
function test() {
  return "bah";
}

types.ts

export type Boo = "bah";
package.json
{
  "name": "repro",
  "version": "1.0.0",
  "main": "index.js",
  "scripts": {
    "test": "echo \"Error: no test specified\" && exit 1"
  },
  "author": "",
  "license": "ISC",
  "description": "",
  "devDependencies": {
    "dependency-cruiser": "^16.6.0",
    "typescript": "^5.6.3"
  },
  "dependencies": {
    "left-pad": "^1.3.0"
  }
}
jsconfig.json
{
  "compilerOptions": {
    "strict": true,
    "checkJs": true,
    "allowJs": true,
    "allowImportingTsExtensions": true,
    "noImplicitAny": true,
    "moduleResolution": "node",
    "module": "ES2022",
    "target": "ES2022"
  },
  "include": ["**/*.js"],
}

Run TypeScript, validate it works (try changing return):

npx tsc -p jsconfig.json 

Initialize depcruise (use current directory):

npx depcruise --init
# set tsPreCompilationDeps to true
npx depcruise . --output-type dot | dot -T svg > dependency-graph.svg

dependency-graph

Context

I am trying to understand dependencies on types.

Your Environment

@sverweij sverweij added the enhancement this will make dependency-cruiser sweeter label Nov 11, 2024
@sverweij
Copy link
Owner

Hi @louwers - thanks for bringing this up!

At this time dependency-cruiser does not support annotations in comments like the ones that have been around for years (e.g. /** @type {import('thing').ISomeInterface} */, or flow annotations) or these new ones.

When I last looked at it (~5.5 years ago) the various parsers didn't emit them in their abstract syntax tree output in a structured fashion (i.e. text only) or didn't emit them at all (e.g. acorn, swc and the typescript-eslint parser strip all comments). That would mean I'd have to write a parser for it - or bring in another library (e.g. @ajaff/tsutils (whose author also contributed to dependency-cruiser) - or these days maybe ts-api-utils which seems to be its successor). Also, at that time typing-in-jsdoc was a niche use case as far as I could see - so I decided to put it aside and work on other things.

This might be different today: a quick check on today's MS typescript compiler output learns it emits something structured for jsdoc comments in its AST + some larger projects went the jsdoc-typing-in-js route, making it less of a niche feature.

... so I'll have a look again. I can't commit to a timeline a.t.m. though.

@louwers
Copy link
Author

louwers commented Nov 12, 2024

Ah I assumed some kind of API from TypeScript was used for parsing type dependencies.

some larger projects went the jsdoc-typing-in-js route, making it less of a niche feature

Yes and I it looks the TypeScript team changed their stance, facilitating the jsdoc-typing-in-js approach even more going forward:

"Our approach to JSDoc has evolved since this issue was created. (...) Typescript is the largest processor of JSDoc at this point, and TS users who want typing are probably the most prolific authors of it."

microsoft/TypeScript#27387 (comment)

... so I'll have a look again. I can't commit to a timeline a.t.m. though.

Cool. Thanks for the quick reply. Very neat project!

sverweij added a commit that referenced this issue Nov 17, 2024
## Description

- detects [jsdoc @import
imports](https://devblogs.microsoft.com/typescript/announcing-typescript-5-5/#the-jsdoc-import-tag)
added in TS5.5
- adds a new `jsdoc`, `jsdoc-import-tag` dependency types
- adds unit tests
- dog foods the `@import` jsdoc tag
- [x] feature switches it in the configuration (otherwise it would be a
breaking change) -
- [x] feature switch true implies `parser` set to `tsc`
- [x] document it (and add caveats (TS 5.5 and up - `parser` attribute
set to `tsc`)
- [x] adds e2e test
- 🏕️ disables the yarn berry integration test - for #reasons it now acts
up on the ci (node 23 issue? Something else?), where it runs perfectly
fine on local machine - something to figure out later.

Future work:
- also detect jsdoc-bracket-import (`/** @type {import('yadda').bla}
*/`) statements in other jsdoc tags - see [typescript jsdoc
reference](https://www.typescriptlang.org/docs/handbook/jsdoc-supported-types.html)
for a list => likely better in a _separate PR_.

## Motivation and Context

Addresses #964 

## How Has This Been Tested?

- [x] green ci
- [x] additional automated non-regression 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)
- [x] New feature (non-breaking change which adds functionality)
- [ ] Breaking change (fix or feature that would cause existing
functionality to change)
@sverweij
Copy link
Owner

@louwers I've published [email protected] a few minutes ago - it recognizes @import tags in jsdoc comments. As compared to the version you tested it takes one extra step: add the detectJSDocImports option in .dependency-cruiser.js, set it to true and Bob's your uncle.

I'll try and add support for the earlier 'bracket style' imports before releasing this with the latest tag on npmjs so we don't introduce a breaking change between two latest versions.

Thanks again for the very pro-active testing & feedback in the PR!

@louwers
Copy link
Author

louwers commented Nov 17, 2024

Thanks for implementing this, and nice to see you actually have a use for it too. 👍

@louwers louwers closed this as completed Nov 17, 2024
sverweij added a commit that referenced this issue Dec 1, 2024
## Description

- adds recognition for the 'older' bracket style imports in jsdoc 
(so things like `/** @type {import('thing')} */`, `/** @typedef
{import('thing')} Thing */`, `/** @param {import('thing')} pParameter
blablabla */`, ...)
- updates documentation to reflect that it isn't a 'future feature'
anymore
- 🏕️ drive-by: updates jsdoc type references in dependency-cruiser's own
sources

TODO
- [x] Add support for more exotic ways to import (e.g. `/** @type
{Partial<import('bla')>} */`, `/**
@type{boolean|import('bla')|import('ble')} */`, `/**
{Set<string,Partial<import('bla')>} */`, ...)
- [x] Verify with [the list of jsdoc tag types typescript
supports](https://www.typescriptlang.org/docs/handbook/jsdoc-supported-types.html)
there's no other corner cases that can bite us.
- [x] so... we currently seem to occasionally resolve these type only
imports to the real implementation. Is that an issue?

> [!NOTE]
> an iteration of this pull request has been published [as a beta on
npmjs](https://www.npmjs.com/package/dependency-cruiser/v/16.7.0-beta-2)


## Motivation and Context

- necessary to be more complete than just `@import` tags (see #964 and
#965)
- needs to be introduced at the same time as those to prevent a breaking
change


## 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)
- [x] New feature (non-breaking change which adds functionality)
- [ ] Breaking change (fix or feature that would cause existing
functionality to change)
@sverweij
Copy link
Owner

sverweij commented Dec 1, 2024

Dependency-cruiser recognizes both the @import style as well as the older 'bracket style' imports as of version 16.7.0 which just landed on npm.

release notes: https://github.com/sverweij/dependency-cruiser/releases/tag/v16.7.0

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement this will make dependency-cruiser sweeter
Projects
None yet
Development

No branches or pull requests

2 participants