import-js / eslint-plugin-import

ESLint plugin with rules that help validate proper imports.
MIT License
5.47k stars 1.56k forks source link

`no-unresolved` is not aware of `exports` definition in `package.json` #1810

Open ignatiusreza opened 4 years ago

ignatiusreza commented 4 years ago

From this announcement

Package entry points and the package.json “exports” field

There are now two fields that can define entry points for a package: “main” and“exports”. The “main” field is supported in all versions of Node.js, but its capabilities are limited: it only defines the main entry point of the package. A new package.json field “exports” can also define the main entry point, along with subpaths. It also provides encapsulation, where only the paths explicitly defined in “exports” are available for importing. “exports” applies to both CommonJS and ES module packages, whether used via require or import.

Might need to be fix in resolve package..

ljharb commented 4 years ago

Indeed, that’s where it needs to be fixed.

However, for back compat, you should have a file at the location the exports key would otherwise resolve to anyways - so even when it’s fixed in resolve, you’d still want it fixed now.

inlined commented 3 years ago

Will this be resolved any time soon? We're going to have to yank no-unresolved from our Google code samples because it can no longer handle our SDKs.

ljharb commented 3 years ago

@inlined why would your Google SDKs be using exports, without main, in a way that's not backwards-compatible??

inlined commented 3 years ago

We are using main. We also use exports for submodule paths so we can omit the lib folder generated by TypeScript.

For example, ./lib/v2/providers/https.js is exported at ./v2/https. See github.com/firebase/firebase-functions for more info.

ljharb commented 3 years ago

Right - but that's not backwards-compatible to pre-exports node. However, if you created an actual ./v2/https.js file that re-exported that provider, then not only would your package be backwards-compatible, but you wouldn't be blocked on resolve getting exports support.

inlined commented 3 years ago

We control our execution environment and do not need to be backwards compatible to versions that don't support exports (every supported LTS of Node supports exports). Also, putting a root export would muddy project structure. It is quite common to have a /src and /lib folder for typescript projects. With exports it is quite understandable to remove lib from documented import paths.

ljharb commented 3 years ago

@inlined i'm not "of the opinion that exports should not use renaming features", i'm of the opinion that you have a really simple workaround while you wait.

None of my "opinions" are delaying exports support - the work is difficult and nobody else is doing it, and I have limited time. It will be done eventually.

ljharb commented 3 years ago

@inlined if google wants to help make it happen faster, please feel free to visit https://github.com/browserify/resolve?sponsor=1

vvo commented 2 years ago

For what it's worth, here's how I solved this situation for a package of mine.

The import style of the library is:

import { fn } from "mylib/next";
import { fn } from "mylib/express";

I updated my package.json build steps with:

{
    "prepublishOnly": "npm run build && cp next/dist/* next/ && cp express/dist/* express/",
    "postpublish": "rm next/*.d.ts next/*.js next/*.map next/*.mjs && rm express/*.d.ts express/*.js express/*.map express/*.mjs"
}

What this does is make copies of the build output files right before npm publish and remove them right after. A bit hacky but it definitely works and will ensure the package works well even on bundlers not supporting the exports: {} field of package.json.

Thanks to the maintainers of eslint-plugin-import for the very hard work they do.

ljharb commented 2 years ago

@vvo you can also .gitignore those files (don't forget to make an .npmignore and unignore them there) and avoid the need to remove them afterwards.

vvo commented 2 years ago

Indeed, but I don't want to see these files in my editor (VSCode), they would still appear as grayed out I guess. But I will still add them to gitignore so they never get published committed.

ljharb commented 2 years ago

@vvo gitignore is so they don't get committed; you DO want them published, which means you have to have a .npmignore that duplicates your gitignore but removes the lines that ignore build output.

vvo commented 2 years ago

Thanks updated my comment to add committed. Also, I am using the "files: ["dist", "express", "next"]" setting of npm.

danielweck commented 2 years ago

Hi all, FYI I am successfully using this tiny ESLint import resolver to support ESM modules imports via package.json exports map:

https://gist.github.com/danielweck/cd63af8e9a8b3492abacc312af9f28fd

Duplicate issue? https://github.com/import-js/eslint-plugin-import/issues/1868

bertho-zero commented 2 years ago

I created a simple resolver that works thanks to enhanced-resolve:

resolver.js

'use strict';

const fs = require('graceful-fs');
const path = require('path');
const { builtinModules } = require('module');
const enhancedResolve = require('enhanced-resolve');
const CachedInputFileSystem = require('enhanced-resolve/lib/CachedInputFileSystem');

const builtins = new Set(builtinModules);

const nodeFileSystem = new CachedInputFileSystem(fs, 4000);
const defaultResolver = enhancedResolve.create.sync(opts());

function resolve(source, file, config) {
  if (builtins.has(source)) {
    return { found: true, path: null };
  }

  try {
    const resolver = config ? enhancedResolve.create.sync(opts(config)) : defaultResolver;
    const result = resolver(path.dirname(file), source);

    return { found: true, path: result };
  } catch (e) {
    return { found: false };
  }
}

function opts(config) {
  return Object.assign({
    fileSystem: nodeFileSystem,
    conditionNames: ['node'],
    extensions: ['.mjs', '.js', '.json', '.node'],
    preferRelative: true,
  }, config);
}

module.exports = {
  interfaceVersion: 2,
  resolve,
};

Usage (eslint.config.js):

// without config
module.exports = {
  // ...
  settings: {
    'import/resolver': path.resolve(__dirname, './resolver')
  },
  // ...
};

// with `enhanced-resolve` config
module.exports = {
  // ...
  settings: {
    'import/resolver': {
      [path.resolve(__dirname, './resolver')]: {
        extensions: ['.js', '.ts']
      }
    }
  },
  // ...
};
linguofeng commented 2 years ago

Any updates?

DamienCassou commented 2 years ago

I workaround the issue by using no-unresolved's ignore option:

{
  // workaround for
  // https://github.com/import-js/eslint-plugin-import/issues/1810:
  "import/no-unresolved": ["error", { ignore: ["prosemirror-.*"] }],
}
piranna commented 2 years ago

@bertho-zero it looks similar to https://gist.github.com/danielweck/cd63af8e9a8b3492abacc312af9f28fd, can you create a npm package from it?

cyco130 commented 2 years ago

I just published a resolver package that solves this issue using resolve.exports: https://www.npmjs.com/package/eslint-import-resolver-exports

It's currently beta but seems to work for most of my projects. Feedback appreciated.

DamienCassou commented 2 years ago

It's currently beta but seems to work for most of my projects. Feedback appreciated.

do you have examples that we could look at? I don't understand how to configure the resolver to get the same behavior as the one from the webpack resolver.

cyco130 commented 2 years ago

do you have examples that we could look at?

Currently what's in the readme is all I have. I think it's best to use this as a fallback in addition to other resolvers as it only supports main, module, exports and nothing else. The readme shows how to use it with TypeScript resolver for example. You should probably keep using Webpack's resolver too.

Check the resolve.exports docs for configuration options.

JounQin commented 2 years ago

You can use https://github.com/import-js/eslint-import-resolver-typescript which supports exports in package.json instead.

ljharb commented 2 years ago

That should only be a suggestion for TS users; the real solution here is for resolve to add support for exports.

JounQin commented 2 years ago

That should only be a suggestion for TS users; the real solution here is for resolve to add support for exports.

Of course, I didn't close this issue. 🤣

stagas commented 2 years ago

I just published a resolver package that solves this issue using resolve.exports: https://www.npmjs.com/package/eslint-import-resolver-exports

It's currently beta but seems to work for most of my projects. Feedback appreciated.

Thank you @cyco130, that solved it for me.

conartist6 commented 1 year ago

I'm proposing to fix this by writing a new resolver. I've been experimenting recently (with great success, I feel) in making code sync/async agnostic by making aggressive use of the strategy pattern, which allows you to write functionally pure code which makes requests to and receives responses from a stateful core. Because that request/response mechanism is yield, the core is able to complete the request synchronously, or it may leave the generator suspended while it waits for async lookups to complete.

Would anyone be interested in collaborating me on a project like that if it would fix this issue once and for all?

ljharb commented 1 year ago

Fixing resolve would be of much larger impact than a new resolver.

cyco130 commented 1 year ago

Hi @conartist6, I already have a working resolver I shared in my comment. I gladly accept contributions :)

nia072011 commented 1 year ago

Why not use createRequire(importer).resolve(importee,{paths:[]}) directly? In order to support configuring extensions, try resolve importee.[ext] and importee/index.[ext] again.

ljharb commented 1 year ago

@nia072011 that wouldn't work in older node versions, and still wouldn't support ESM. It's not a terrible workaround tho pending the resolve implementation.

magnusriga commented 10 months ago

Thanks @cyco130 et al.

Just so I have not misunderstood anything, I am getting an ESLint error when importing a TS component B into a JS component A, when B is only exported via exports in package.json. If I convert A into TypeScript, the error disappears (I am guessing that is because eslint-import-resolver-typescript supports the exports field).

This is the error I get:

Unable to resolve path to module 'ui/button'. eslint(import/no-unresolved)

To be clear, this works:

But, this does not work:

Is that still the issue the rest of you see as well?

ljharb commented 10 months ago

@magnusriga this can be easily solved by making sure that the actual path to the file is present in the LHS of exports, so that pre-exports node (and resolve, and thus this plugin) can find it also;.

magnusriga commented 10 months ago

@ljharb Thank you for the reply. Do you mean I do not need the new eslint-import-resolver-exports package?

This is my exports field in package.json (I output js to .dist and types to ./types):

image

With the above, all imports are accepted by ESLint except when I try to import a TS component into a JS component (I am using eslint-import-resolver-typescript).

ljharb commented 10 months ago

in that case you only have native ESM, which we don’t support yet.

magnusriga commented 10 months ago

I am not as knowledgable here, but what you are saying is that when I am using import { B } from 'acme/ui' I am using ESM style imports, and not CommonJS (require). And currently, a workaround (like the new eslint-import-resolver-exports package) is needed to allow TS files to import JS files when the latter is exported with exports (like in my screen shot above).

Is that correct?

Thanks a lot.

ljharb commented 10 months ago

The import style is irrelevant; the issue is what format the thing you're importing is using.

magnusriga commented 10 months ago

@ljharb Thanks for the clarification. Do you think eslint-plugin-import will support importing ESM components in the near future, or should we expect to rely on alternatives for the next few months?

As a side note, based on your answer I was a bit surprised that I did not get the ESLint error when importing an ESM JS component into another JS component. The error only appeared when importing TS into JS. That's extra bizarre, as I thought it looked in the dist folder (as dictated by exports in package.json [screen shot in my previous post]) where everything is already transpiled to JS. Perhaps one of my other plugins is enabling that type of import (I use @vercel/styleguide, which comes with a few packages built-in).

ljharb commented 10 months ago

This plugin can't support exports, nor native ESM, until resolve does; I have no timeline for when resolve will support that, but I continue to hope it's "soon".

conartist6 commented 10 months ago

What's it been so far, three or four years?

ljharb commented 10 months ago

Yup. Doing unpaid volunteer work doesn't create a lot of free time.

conartist6 commented 10 months ago

I'm just gonna build it, unless anyone objects. Now is a good moment for me.

ljharb commented 10 months ago

You've always been welcome to, but unless it's in the resolve package it won't have the same benefit, I'm afraid.

conartist6 commented 10 months ago

Could you clarify what you mean?

I think of this as a case that demands competive compatibility, but so long as I'm doing a full rewrite I will have little desire to release my work under their name (even if it were possible).

ljharb commented 10 months ago

it is, as I’m the sole maintainer of resolve, but a full rewrite isn’t needed anyways.

conartist6 commented 10 months ago

I'd rather not build it in a way that all the important logic is duplicated between the sync and async pathways. My aim would be define the resolution algorithm once and only once, and generator functions are the means to do it.

conartist6 commented 10 months ago

There aren't any versions of node that support exports but don't support generators, right? In other words, old versions of node would continue to use your resolve package.

My primary concern would be ensuring that users who are writing code in the most modern and correct style have readily available tools to help them.

conartist6 commented 10 months ago

ESLlint 4.19.1 (now six years old) requires node >4, which already supported ES6. Runtime module resolution in old node versions doesn't know about exports anyway, so as I say for those users there would be no need to upgrade resolve.

ljharb commented 10 months ago

The resolution needs to be done regardless of the current node version to be usable here.

conartist6 commented 10 months ago

I'm just trying to figure out where the requirement for the resolver to be implemented in es3 is coming from

ljharb commented 10 months ago

From the node versions it needs to support. Generators are garbage anyways and should be avoided, imo.