Special-casing non-standard import syntax in no-extraneous-dependencies

[simonwiles]

I'm using eslint-plugin-import with Parcel v2, which allows the definition of named pipelines such that I can import assets using a line like

import logoPng from "data-url:./logo.png";

I can make these imports resolvable by eslint-plugin-import by using the eslint-import-resolver-parcel2 package, and it works fine.

However, if I want to load an asset from an package installed in node_modules, e.g.:

import layersPng from "url:leaflet/dist/images/layers.png";

I get an error:'url:leaflet' should be listed in the project's dependencies. Run 'npm i -S url:leaflet' to add it.

Likewise if I try:

import layersPng from "url:./node_modules/leaflet/dist/images/layers.png";

then I get 'url:.' should be listed in the project's dependencies. Run 'npm i -S url:.' to add it.

I think the difference is that the resolved file name passes the external check here:
https://github.com/benmosher/eslint-plugin-import/blob/master/src/rules/no-extraneous-dependencies.js#L125-L127

in the first case, and not in the second?

Is there anything I can do about this? Perhaps in the eslint-import-resolver-parcel2 plugin?

Related: an ignore facility à la #903 would provide a useful workaround in this case.

[ljharb]

I’m not sure why anything in this plugin would be parsing the “data-“ part, so i assume it’s the parcel2 resolver that’d need to handle it.

however, I’d still caution against using any bundler-specific syntax - there’s a reason webpack-specific syntax is broadly discouraged, and it’s not because of webpack.

[simonwiles]

eslint-plugin-import is parsing, e.g., url:leaflet/dist/images/layers.png and then raising an error because url:leaflet is not in package.json#dependencies.

[ljharb]

ah, i misunderstood and thought the source was data-url: and the plugin was seeing url:.

I still think that if the parcel2 resolver was catching it, the import plugin wouldn't have to, so that's where the fix should be.

[simonwiles]

I'm not entirely sure I understand, I'm sorry. The parcel2 resolver itself doesn't seem to be directly involved. The eslint-import-resolver-parcel2 plugin implements the parcel2 resolution algorithm to test whether the import can be successfully resolved, and this bit seems to work fine. How would the plugin (or the parcel2 resolver itself) handle this? The import statement would need to be skipped somewhere here, I think?:

https://github.com/benmosher/eslint-plugin-import/blob/ff50964df1234515bc6f7969b90d87ada1982008/src/rules/no-extraneous-dependencies.js#L119-L130

The value of node here looks like:

node: <ref *1> Node {
  type: 'Literal',
  start: 220,
  end: 256,
  loc: SourceLocation {
    start: Position { line: 5, column: 17 },
    end: Position { line: 5, column: 53 }
  },
  range: [ 220, 256 ],
  value: 'url:leaflet/dist/images/layers.png',
  raw: '"url:leaflet/dist/images/layers.png"',
  parent: Node {
    type: 'ImportDeclaration',
    start: 203,
    end: 257,
    loc: SourceLocation { start: [Position], end: [Position] },
    range: [ 203, 257 ],
    specifiers: [ [Node] ],
    source: [Circular *1],
    parent: Node {
      type: 'Program',
      start: 0,
      end: 10148,
      loc: [SourceLocation],
      range: [Array],
      body: [Array],
      sourceType: 'module',
      comments: [Array],
      tokens: [Array],
      parent: null
    }
  }
}

but the resolver plugin returns only

{
  found: true,
  path: resolveSync(newSource, {
    basedir: path.resolve(),
    extensions: [...defaultExtensions, ...config.extensions],
  }),
}

Can this be done in a resolver plugin?

[ljharb]

ah, hmm. i think you're right, that this rule is pretty tightly coupled to package.json and doesn't use the resolver for that.

Currently it calls into https://github.com/benmosher/eslint-plugin-import/blob/ff50964df1234515bc6f7969b90d87ada1982008/src/core/importType.js#L100, which presumably returns external. There's a setting import/external-module-folders to add things to what's considered external, but in this case you want this path to be considered internal, presumably.

Would specifying the import/internal-regex setting to include paths that start with url: work for you?

[simonwiles]

I'm trying to work out how eslint-import-resolver-webpack handles webpack loaders of the form import 'file!./whatever' (see here), as it should be basically the same problem. Looking through the code it's not obvious to me. Or does no-extraneous-dependencies struggle with these too?

[ljharb]

I agree it should be the same problem. I don't see any test cases for it, so I'm not really sure if it does handle it. However, since using webpack-specific loader syntax in files is widely considered a bad practice (we even have a rule forbidding it), it's likely folks just haven't run into the problem.

Why can't parcel2 figure out it's an image from the filesystem, as opposed to using something in the specifier?

[simonwiles]

The named pipelines allow implementing different behaviours -- there's url:, data-url:, and bundle-text: by default. Right now I'm using bundle-text: to transclude CSS assets into a bundled .js files for webcomponents (more-or-less exactly as in the example in the Parcel docs linked above). I want to author my CSS in separate files (and transclude CSS from third-party packages) but distribute a single .js file for (each of) my component(s).

[simonwiles]

The problem could be addressable if resolver plugins could modify the value property of the nodes that are passed to no-extraneous-dependencies (as reported in my comment above). These objects have both value and raw properties, but it's not clear to me under what circumstances these would be different anyway?

[ljharb]

Hmm.

https://github.com/benmosher/eslint-plugin-import/blob/ff50964df1234515bc6f7969b90d87ada1982008/src/rules/no-extraneous-dependencies.js#L130 suggests that if the resolver returns null for the path, it won't be warned.

I think there's a bigger issue here, though, around URL protocols, like file:, data:, node:, etc, that might need a larger solution.

In other words, https://github.com/benmosher/eslint-plugin-import/blob/ff50964df1234515bc6f7969b90d87ada1982008/src/core/importType.js#L100 doesn't seem like it can properly handle file/data/blob/node protocols on a specifier. Would you be willing to work on that first, with a PR, with the idea that the solution for the node resolver would also work for the webpack and Parcel2 resolvers?

[simonwiles]

Hmm.

https://github.com/benmosher/eslint-plugin-import/blob/ff50964df1234515bc6f7969b90d87ada1982008/src/rules/no-extraneous-dependencies.js#L130

suggests that if the resolver returns null for the path, it won't be warned.

Yeah, but that effectively just disables no-extraneous-dependencies -- in an ideal world something like this

import layersPng from "url:leaflet/dist/images/layers.png";

would raise a warning if leaflet isn't in package.json#dependencies.

Would you be willing to work on that first, with a PR, with the idea that the solution for the node resolver would also work for the webpack and Parcel2 resolvers?

Yeah, I'll take a stab at it. At first glance it looks like it'll just be a case of some sting manipulation on the name in typeTest before it's passed to isAbsolute etc.? Grokking the test suite and writing the tests will take longer :)

using webpack-specific loader syntax in files is widely considered a bad practice

Fwiw, I do agree with this in principle. I'm bundling webcomponents for deployment by non-developers, though, and I really want to make it possible with a single .js file. Using parcel (or webpack or indeed any bundler)-specific features breaks the ability to use the unbundled ESM code, which is the no. 1 argument against, especially as I'm encouraging the use of unpkg so an ESM build could be used with a single <script> tag, but I know my end-consumers are going to try to download the files and self-host whatever I suggest :(

[ljharb]

You're totally right that "disable the rule" isn't the best path.

I think it's just some string manipulation, but it's likely that it needs a URL parser of some kind, so that it can match node's own resolution algorithm.

[simonwiles]

I'm not 100% au fait with what's available in a Node.js environment, so I'll do a bit of reading and then take a first pass at this.

[dryoma]

Hey everyone

So I understand this will no longer work and has to be rewritten?

import 'expose-loader?exposes[]=$&exposes[]=jQuery!jquery';

Using webpack 4 on some of the projects, and getting this error from the linter for every instance like the above:

error  'expose-loader?exposes=_!underscore' should be listed in the project's dependencies. Run 'npm i -S expose-loader?exposes=_!underscore' to add it import/no-extraneous-dependencies

[ljharb]

@dryoma i don’t believe that webpack-only syntax would ever have worked with this plugin; it’s a best practice to avoid it. The webpack resolver would have to be updated to handle that syntax, and I’m happy to review a PR that does that.

[dryoma]

@ljharb It used to work for us prior to 2.20.1. At least it didn't trigger eslint errors, so maybe such values were simply ignored.

[ljharb]

That seems more likely. I’ll look into it.

[ljharb]

@dryoma it seems to work just fine in the latest version; can you try with the latest version of the plugin?

I'll close this with a test case, but will reopen if needed.