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

Using Typescript 5.0's array extends in tsconfig causes an error while linting #2751

Closed
NutchapolSal opened this issue Mar 29, 2023 · 23 comments

Comments

@NutchapolSal
Copy link

Typescript 5.0 now supports combining TSConfigs using array syntax in extends

Error log:

TypeError [ERR_INVALID_ARG_TYPE]: The "path" argument must be of type string. Received an instance of Array
Occurred while linting G:\gits\NutchapolSal\musicbot\botCacheInit.js:1
Rule: "import/namespace"
    at new NodeError (node:internal/errors:393:5)
    at validateString (node:internal/validators:163:11)
    at Object.join (node:path:429:7)
    at loadTsconfig (G:\gits\NutchapolSal\musicbot\node_modules\tsconfig-paths\lib\tsconfig-loader.js:93:39)
    at loadSyncDefault (G:\gits\NutchapolSal\musicbot\node_modules\tsconfig-paths\lib\tsconfig-loader.js:40:18)
    at tsConfigLoader (G:\gits\NutchapolSal\musicbot\node_modules\tsconfig-paths\lib\tsconfig-loader.js:26:22)
    at readTsConfig (G:\gits\NutchapolSal\musicbot\node_modules\eslint-plugin-import\lib\ExportMap.js:804:521)
    at isEsModuleInterop (G:\gits\NutchapolSal\musicbot\node_modules\eslint-plugin-import\lib\ExportMap.js:806:277)
    at ExportMap.parse (G:\gits\NutchapolSal\musicbot\node_modules\eslint-plugin-import\lib\ExportMap.js:798:233)
    at ExportMap.for (G:\gits\NutchapolSal\musicbot\node_modules\eslint-plugin-import\lib\ExportMap.js:797:201)

This also effects import/namespace, import/named, import/default, import/no-named-as-default, and import/no-named-as-default-member.

@ljharb
Copy link
Member

ljharb commented Mar 29, 2023

tsconfig-paths doesn't support this yet, and even if they added that support, they'd need to backport it to v3 for us to be able to use it.

@WoodyWoodsta
Copy link

See dividab/tsconfig-paths#245, which was released in tsconfig-paths v4. Either:

  1. tsconfig-paths backports support to v3, or
  2. eslint-plugin-import upgrades tsconfig-paths to v4

I'm unable to find what the breaking changes are in v4, so I don't know how much of an issue it would be to upgrade in this package.

@WoodyWoodsta
Copy link

Perhaps:

Ignore --project/-P CLI flag when explicit options are passed to register

If this is the only breaking change in tsconfig-paths I don't see why this is a problem for this project, since it should not be concerned about tsconfig-path's CLI

@ljharb
Copy link
Member

ljharb commented Mar 31, 2023

@WoodyWoodsta its a nonstarter, see #2447, so hopefully they go with option 1.

@WoodyWoodsta
Copy link

Ouch - not to bring that discussion over here, but maintaining Node v4 support is not something I've come across before. That is your decision though, but I don't think that tsconfig-paths has any obligation to backport to v3 given the fix is for new major releases of a tool. It would be nice, but I'd expect the support for TS 5 to be limited to tsconfig-paths v4.

@ljharb
Copy link
Member

ljharb commented Apr 1, 2023

Nobody has any obligation to do anything :-) i think their v4 platform drop was a mistake, and another park forward is for v4 to restore node 4 support (which it could do in a semver minor).

If tsconfig-paths maintainers would accept a PR restoring that support, I’d be happy to author it, so this plugin could update to v4.

@WoodyWoodsta
Copy link

If tsconfig-paths maintainers would accept a PR restoring that support, I’d be happy to author it, so this plugin could update to v4.

@ljharb Open an issue then.

@ljharb
Copy link
Member

ljharb commented Apr 1, 2023

Yep, that’s indeed the kind of entitlement and laziness that explains why forks aren’t sustainable.

@WoodyWoodsta
Copy link

I'm not sure I understand?

@ljharb
Copy link
Member

ljharb commented Apr 1, 2023

It’s something you want, and I’ve given you a path to getting what you want - and instead of you just filing the issue, you said “psh, you do it”.

@WoodyWoodsta
Copy link

I think that's the misunderstanding. It's the decision you've made, as is your full right, but still your decision, not mine. You have a PR open with 90+ comments, 25 participants all of whom are not in support of your proposition. So no, I don't think I'm practicing any entitlement or laziness. Rather, I've spent a fair amount of time attempting to guide you towards a different perspective which, as unbelievable as it appears to you, is not an unreasonable perspective and is in the interest of the community you lead. 🤷

I'm not really interested in putting any further effort into this sort of a cause. My organisation will see if eslint-plugin-i is suitable, otherwise we'll forfeit linting imports altogether.

@ljharb
Copy link
Member

ljharb commented Apr 1, 2023

Good luck to you.

@knpwrs
Copy link
Contributor

knpwrs commented Apr 4, 2023

Adding this to my package.json resolves the issue for me:

  "overrides":{
    "tsconfig-paths": "^4.0.0"
  }

@yorch
Copy link

yorch commented Apr 6, 2023

For yarn it would be resolutions like:

  "resolutions":{
    "tsconfig-paths": "^4.0.0"
  }

@ljharb
Copy link
Member

ljharb commented Apr 6, 2023

Using overrides/resolutions is a perfectly reasonable approach, assuming the breaking changes don't affect our usage of the package.

@so1ve

This comment was marked as spam.

@so1ve

This comment was marked as off-topic.

@ljharb

This comment was marked as resolved.

@mlippert

This comment was marked as off-topic.

@ljharb

This comment was marked as resolved.

@NutchapolSal

This comment was marked as abuse.

@WoodyWoodsta

This comment was marked as off-topic.

@so1ve

This comment was marked as abuse.

@import-js import-js locked as too heated and limited conversation to collaborators Jun 30, 2023
@ljharb ljharb closed this as completed in 48fec35 Dec 14, 2023
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Development

No branches or pull requests

7 participants