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

import/no-unused-modules can't recognize loading rules with moduleSuffixes #2698

Closed
nzaytsev opened this issue Jan 30, 2023 · 20 comments
Closed

Comments

@nzaytsev
Copy link

nzaytsev commented Jan 30, 2023

I use this rule in my multiplatform app and I load specific parts of code by moduleSuffixes. And I get warnings in platform-specific modules.

my .tsconfig

{
  "compilerOptions": {
    "jsx": "react",
    ...
    "moduleResolution": "node",
    "moduleSuffixes": [".p2", ".connect", ""]
  },
  "include": ["src/**/*", "jest/*"],
  "exclude": ["node_modules"]
}

my webpack config

export const getBaseConfig = (
  options: IWebpackConfigOptions,
): Configuration => {
  // target is launching platform name 
  const {target, isDev} = options
  /*...*/
  const baseConfig: Configuration = {
    entry: {/*...*/},
    stats: {
      children: true,
    },
    cache: true,
    optimization: {/*...*/},
    resolve: {
      extensions: [`.${target}.ts`, '.ts', '.tsx', '.js', '.jsx'],
      alias: {
        // allows to import from 'src/....'
        src: srcDir,
      },
    },
    /*...*/
  }
  if (target === 'connect') {
    plugins.push(...getCloudWebpackPlugins(isDev))
  }
  if (target === 'p2') {
    plugins.push(...getServerWebpackPlugins())
  }

  if (isDev) {
    baseConfig['devServer'] = getDevserverConfig(target)
  }
  return baseConfig
}

then I import some specific module from shared code as

  import {myModule} from './my-module'

where module is defined as

./my-module.p2.ts

  // this export is recognized as used
  export const myModule = () => 'p2'

./my-module.connect.ts

  // here I have to add `eslint-disable-next-line import/no-unused-modules`
  export const myModule = () => 'connect'

I see that it's an unusual way of importing configured by webpack, but if the import/no-unused-modules rule had ignorePattern option, I would pass it as '*.connect.ts' and it would be enough

@ljharb
Copy link
Member

ljharb commented Jan 30, 2023

I'm not familiar with moduleSuffixes - what does this do?

@nzaytsev
Copy link
Author

@ljharb, It can help to import platform-specific module not using conditions. as you can see, I import module without suffix

 import {myModule} from './my-module'

Then, if I build my App with arg target='p2', myModule will be imported from './my-module.p2.ts', and if target='connect', myModule will be imported from './my-module.connect.ts'

this way is also used in ReactNative by default to import system-specific code
https://www.typescriptlang.org/tsconfig#moduleSuffixes

@ljharb
Copy link
Member

ljharb commented Jan 30, 2023

Interesting. How can the linter know that every potential suffix is used or not? or are you suggesting that every moduleSuffix should be considered "used" if it exists?

@nzaytsev
Copy link
Author

I have no idea how to resolve it correctly, because import/resolver of .eslint has similar issue (it resolves only the first extension from the list, in my case it's '.p2.ts'), but I'd like to use ignore patterns to don't check this rule in suffixed files

@nzaytsev
Copy link
Author

JFYI. my eslint settings:

"settings": {
    "react": {
      "version": "17.0.2"
    },
    "import/parsers": {
      "@typescript-eslint/parser": [".ts", ".tsx"]
    },
    "import/resolver": {
      "node": {
        "extensions": [".p2.ts", ".connect.ts", ".js", ".jsx", ".ts", ".tsx"]
      },
      "typescript": {
        "alwaysTryTypes": true,
        "project": "ui/tsconfig.json"
      }
    },
    "import/ignore": ["node_modules"]
  }

@ljharb
Copy link
Member

ljharb commented Jan 30, 2023

To be clear, .p2.ts isn't an extension, and it should either be erroring there or only selecting the .ts - it's a bug that the config accepts those. Extensions are only after the final dot in a filename.

@nzaytsev
Copy link
Author

It's interesting, because if I remove '.p2.ts' and '.connect.ts' from extensions, I will see next situation:
vscode with tsconfig resolver message: module "{pathToModule}/{moduleName}.p2"
and eslint message: Unable to resolve path to module './get-top-inset'.eslint[import/no-unresolved](https://github.com/import-js/eslint-plugin-import/blob/v2.26.0/docs/rules/no-unresolved.md)

@ljharb
Copy link
Member

ljharb commented Jan 31, 2023

yes, because by standard resolution rules, your file is get-top-inset.p2 plus a ts extension, etc.

the node resolver shouldn't support that. However, if you swap the node and typescript resolver ordering, so node is last, i would hope the TS resolver knows about moduleSuffixes. If not, then that's something we should look into.

@nzaytsev
Copy link
Author

nzaytsev commented Jan 31, 2023

the node and typescript resolver ordering

what do you mean?

@ljharb
Copy link
Member

ljharb commented Jan 31, 2023

In your settings object, you specify "node" first, before "typescript" - but you want typescript to do resolution before node does, so you should swap them.

@nzaytsev
Copy link
Author

It still doesn't work

@ljharb
Copy link
Member

ljharb commented Jan 31, 2023

thanks for confirming. @bradzacher, any idea if the TS resolver knows about moduleSuffixes?

@bradzacher
Copy link
Contributor

Doing some digging - it looks like it doesn't.
It's a relatively new TS feature (TS 4.7 - May 2022) and it's only really used by the react native community - so it's not surprising that support hasn't been added yet.

I'd file an issue over at eslint-import-resolver-typescript so they can add support!

@nzaytsev
Copy link
Author

Thank you. Let the link be here
import/no-unused-modules conflicts with typescript moduleSuffixes

@ljharb
Copy link
Member

ljharb commented Jan 31, 2023

ah yes, thanks :-)

@nzaytsev

This comment was marked as spam.

@NikolayFrantsev
Copy link

I've made simple resolver package to solve that (and that) issue.

@nzaytsev
Copy link
Author

@NikolayFrantsev - cool, thank you

@nzaytsev
Copy link
Author

nzaytsev commented Aug 1, 2023

@NikolayFrantsev - I still see the issue
Screenshot_20230801_161912

@JounQin
Copy link
Collaborator

JounQin commented Jan 12, 2024

@JounQin JounQin closed this as not planned Won't fix, can't repro, duplicate, stale Jan 12, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

No branches or pull requests

5 participants