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

Typescript support for moduleResolution node16 #96

Open
k2snowman69 opened this issue Aug 6, 2023 · 14 comments
Open

Typescript support for moduleResolution node16 #96

k2snowman69 opened this issue Aug 6, 2023 · 14 comments

Comments

@k2snowman69
Copy link

As a typescript user when setting moduleResolution to Node16 or NodeNext, I currently get typescript failures from the ignore package.

Due to typescript-eslint@6 upgrading to only support Node16+, I'm unable to support both simultaneously.

For more information on possible fixes see:
https://arethetypeswrong.github.io/?p=ignore

@kaelzhang
Copy link
Owner

Could you provide the use cases or concrete code slices?

Is it only a linting warning about potential issues or are there any real failures?

Here is the test case only for ts definition: https://github.com/kaelzhang/node-ignore/blob/master/test/ts/simple.ts

@k2snowman69
Copy link
Author

Sure and it's a real typescript typing/intellisense failure, not just linting. I ran into this in my own project here:
https://github.com/snowcoders/sortier/pull/2226/files#diff-55dee8ec3daf30391537fdc45f8c2639cca5d3e63faf83671ee7505eeb8fed03R10

I think with typescript + node16 resolution, when you share a single types file with cjs and esm, it seems to assume cjs. In this code, I had to @ts-expect-error because ignore here was the import module and typescript wanted me to use ignore.default(). Obviously that doesn't match your build output so I had to ignore the error to be able to get the program to continue to run.

If you need a repo environment, that project should work, just removed the @ts-expect-error and you should see the same bug. If that doesn't work for you, creating a narrowed reproduction repo wouldn't be difficult, it will just take a few days since the work week is starting.

@kaelzhang
Copy link
Owner

kaelzhang commented Aug 10, 2023

It is really weird that tsc treats packages with/without type: module differently.

  • tsc ./src/lib/is-ignored/index.ts --moduleResolution Node16 failed with type:module in package.json
  • tsc ./src/lib/is-ignored/index.ts --moduleResolution Node16 succeeded with no type:module in package.json
  • tsc ./src/lib/is-ignored/index.ts succeeded

@guoyunhe
Copy link

@kaelzhang I think the only way to fix this is to add a real esm output:

{
  "name": "ignore",
  "version": "5.3.0",
  "main": "index.js",
  "module": "index.mjs",
  "types": "index.d.ts"
}

@kristianmandrup
Copy link
Contributor

I'm getting the error: ignore has no call signatures when using Node16 modules and doing:

import ignore from "ignore";
const ig = ignore();

Looking at the type definitions:

/**
 * Creates new ignore manager.
 */
declare function ignore(options?: Options): Ignore

declare namespace ignore {
  export function isPathValid (pathname: string): boolean
}

export default ignore

It is clear that the last definition of ignore is a namespace which I presume overwrites the previous function declaration?

@kristianmandrup
Copy link
Contributor

I managed to make it work like this:

tsconfig.json

{
  "compilerOptions": {
    "baseUrl": ".",
    "typeRoots": ["./src/@types", "./node_modules/@types"],

src/@types/ignore.d.ts

type Pathname = string;

interface TestResult {
  ignored: boolean;
  unignored: boolean;
}

export interface Ignore {
  /**
   * Adds one or several rules to the current manager.
   * @param  {string[]} patterns
   * @returns IgnoreBase
   */
  add(patterns: string | Ignore | readonly (string | Ignore)[]): this;

  /**
   * Filters the given array of pathnames, and returns the filtered array.
   * NOTICE that each path here should be a relative path to the root of your repository.
   * @param paths the array of paths to be filtered.
   * @returns The filtered array of paths
   */
  filter(pathnames: readonly Pathname[]): Pathname[];

  /**
   * Creates a filter function which could filter
   * an array of paths with Array.prototype.filter.
   */
  createFilter(): (pathname: Pathname) => boolean;

  /**
   * Returns Boolean whether pathname should be ignored.
   * @param  {string} pathname a path to check
   * @returns boolean
   */
  ignores(pathname: Pathname): boolean;

  /**
   * Returns whether pathname should be ignored or unignored
   * @param  {string} pathname a path to check
   * @returns TestResult
   */
  test(pathname: Pathname): TestResult;
}

export interface Options {
  ignorecase?: boolean;
  // For compatibility
  ignoreCase?: boolean;
  allowRelativePaths?: boolean;
}

declare module "ignore" {
  /**
   * Creates new ignore manager.
   */
  export function ignore(options?: Options): Ignore;
}

Now I can use it as:

import { ignore } from "ignore";

const ig = ignore();

@scolladon
Copy link

I also face this issue when trying to upgrade my app to typescript compilation with moduleResolution node16
Let me know if there is anything I can do to help here (data, reproduction environment, etc)

I managed to make it work like this:
@kristianmandrup I tried and it did pass the compilation issues but then my tests failed... I have not dug much to understand why.

@kristianmandrup
Copy link
Contributor

kristianmandrup commented Sep 9, 2024

The above solution works "in principle" to remove the eslint type warning. However it should obviously export default as this matches the underlying code.

import ignore from "ignore";

Via

type Pathname = string;

interface TestResult {
  ignored: boolean;
  unignored: boolean;
}

declare module "ignore" {
  export interface Ignore {
    /**
     * Adds one or several rules to the current manager.
     * @param  {string[]} patterns
     * @returns IgnoreBase
     */
    add(patterns: string | Ignore | readonly (string | Ignore)[]): this;

    /**
     * Filters the given array of pathnames, and returns the filtered array.
     * NOTICE that each path here should be a relative path to the root of your repository.
     * @param paths the array of paths to be filtered.
     * @returns The filtered array of paths
     */
    filter(pathnames: readonly Pathname[]): Pathname[];

    /**
     * Creates a filter function which could filter
     * an array of paths with Array.prototype.filter.
     */
    createFilter(): (pathname: Pathname) => boolean;

    /**
     * Returns Boolean whether pathname should be ignored.
     * @param  {string} pathname a path to check
     * @returns boolean
     */
    ignores(pathname: Pathname): boolean;

    /**
     * Returns whether pathname should be ignored or unignored
     * @param  {string} pathname a path to check
     * @returns TestResult
     */
    test(pathname: Pathname): TestResult;
  }

  export interface Options {
    ignorecase?: boolean;
    // For compatibility
    ignoreCase?: boolean;
    allowRelativePaths?: boolean;
  }

  /**
   * Creates new ignore manager.
   */
  export default function ignore(options?: Options): Ignore;
}

I propose to change the index.d.ts exported by ignore to the above

@scolladon
Copy link

The index.d.ts proposed above works for (unit test with jest and ts-jest and typescript compilation)

@scolladon
Copy link

Do you plan on submitting a pull request @kristianmandrup ?

@kristianmandrup
Copy link
Contributor

Sure. Why not 😊

@kristianmandrup
Copy link
Contributor

Made a PR #136 with this fix

@scolladon
Copy link

I see the PR has been reverted #136

What was the issue ?

@kaelzhang I think the only way to fix this is to add a real esm output:

{
  "name": "ignore",
  "version": "5.3.0",
  "main": "index.js",
  "module": "index.mjs",
  "types": "index.d.ts"
}

Have you tried this approach @kristianmandrup @kaelzhang ?

@kaelzhang
Copy link
Owner

kaelzhang commented Oct 7, 2024

Don't waste your life writing Typescript, try Python instead.

I have used countless programming languages ​​in production environments, and TypeScript is the one I hate the most, not one of. Typescript really sucks.

I really don't have time to waste on it. Can anyone help me solve this issue?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

5 participants