import-js/eslint-import-resolver-typescript

v2.0.0 prefers '.d.ts' files to '.js' files when both exist.

jgerigmeyer opened this issue ยท 12 comments

In v2.0.0, it seems that the change from:

  const extensions = Object.keys(require.extensions).concat(
    '.ts',
    '.tsx',
    '.d.ts',
  );

to:

const extensions = ['.ts', '.tsx', '.d.ts'].concat(
  // eslint-disable-next-line node/no-deprecated-api
  Object.keys(require.extensions),
  '.jsx',
)

...means that if a node module is imported and contains both .js and .d.ts files, the .d.ts file is now resolved instead of the .js one. This results in import/default errors, e.g.:

import React from 'react';
error: No default export found in module (import/default)

Maybe I'm missing something obvious?

Yes, it's just working as expected.
See #31 (comment)

@JounQin would it make sense to document this behavior somewhere in README?

@JounQin Sorry, don't know what you mean by "it's working as expected". Both here and here. I'm having issues with lodash as well, just as in #31.

Screen Shot 2019-10-29 at 16 39 18

In all of these scenarios, there is a default export that always used to work (with esModuleInterop):

// node_modules/dataloader/index.d.ts
declare class DataLoader {}
export = DataLoader;
// node_modules/@types/lodash/snakeCase.d.ts
import { snakeCase } from "./index";
export = snakeCase;

So it's not working as we're expecting.

It's not ES Module default export, it's commonjs, that why the eslint-plugin-import complains about no default export.

And you should issue to eslint-plugin-import for help supporting that instead.

With esModuleInterop.

On top of those already mentioned, import/order also started to raise:

Screen Shot 2019-10-29 at 17 28 05

express: external types in @types/express - seen as "internal"
http-status-codes: own types - seen as "external"

I'm not that well versed in eslint-plugin-import and/or eslint-import-resolver-typescript to understand what is going on - or even where the boundaries are. So can't say yet where the problem is, but will dig deeper. But, as a consumer, it's not working as expected.

Regarding disabling the rules: typescript-eslint/typescript-eslint#154 (comment)

esModuleInterop is TypeScript spec and eslint-plugin-import doesn't understand it, that's why I told you to issue to it instead.

And also, eslint-plugin-import considers foo and @types/foo as different packages, so it's still as expected on eslint-import-resolver-typescript's side.

Please, issue to eslint-plugin-import instead if it's not as expected for you.

Ok. For the import/order, I've raised import-js/eslint-plugin-import#1525 and fixed in import-js/eslint-plugin-import#1526.

For the import/default, where do you think the problem would be? In the parser or the import plugin?

In 1.1.1 reading from the JS source (lodash/snakeCase.js) works fine:

var snakeCase = createCompounder(function(result, word, index) {
  return result + (index ? '_' : '') + word.toLowerCase();
});

module.exports = snakeCase;

In 2.0.0 using the TS export syntax in the type definition (@types/lodash/snakeCase.d.ts) it doesn't work:

import { snakeCase } from "./index";
export = snakeCase;

I've told you the type definition is commonjs format and eslint-plugin-import doesn't treat it as an ES Module then. It doesn't even know any compilerOptions about esModuleInterop.

And it's not regression in my opinion then, it's just the correct behavior. Of course, it could be better to support esModuleInterop for eslint-plugin-import, and there is nothing we can do on side of eslint-import-resolver-typescript.

@JounQin I don't know how you can maintain this project with that attitude. I'm trying to help as you've seen. I'm not saying the problem is in the resolver - I already understood it isn't after having to dig in myself. But all these issues are related to the change in the way the resolver resolves files. You can't expect the rest of the ecosystem to automatically fix itself and just ignore it. As it stands, migrating to 2.0.0 does cause regressions for users.

I've already understood the issue, so I'll move away from this thread as its not helpful.

It is very clear I've answered your latest question at my first comment. It's great to address an issue or even usage question for this project, but I can't understand why I should answer the same question for several times.

And also, I'm not ignoring them, I've answered every comment about this and pinned the most related two issues. If you'd like to help, you can also raise a PR mentioned about this.

And also, we do have a CHANGELOG since v2.0.0, it's a BREAKING CHANGE, that's why we bump the major version.