eslint-plugin-import silently fails if the module is minified

[RPGillespie6]

I have 2 files:

1. test.js

import { bogus } from './bogus.js';

bogus()

2. bogus.js

function foo() {
    console.log('foo');
}

export {
    foo
}

If bogus.js is minified, eslint-plugin-import silently fails:

$ npx eslint js/test.js        # Working case

/home/user/test/js/test.js
  1:10  error  bogus not found in './bogus.js'  import/named

✖ 1 problem (1 error, 0 warnings)

$ npx minify js/bogus.js > bogus2.js
$ mv bogus2.js js/bogus.js
$ npx eslint js/test.js        # Silent failure case
$

Here's my eslint config:

import globals from "globals";
import pluginJs from "@eslint/js";
import importPlugin from "eslint-plugin-import";

/** @type {import('eslint').Linter.Config[]} */
export default [
  importPlugin.flatConfigs.recommended,
  {languageOptions: { globals: globals.browser }},
  pluginJs.configs.recommended,
];

Tested with eslint 8.X and 9.X, issue is present in both. Using eslint-plugin-import:2.31.0

Edit: Actually, some types of minification work. For example:

function a(){console.log("foo")};export{a}; // works
function a(){console.log("foo")}export{a}; // doesn't work

Is this library using regex or something to detect exports instead of the file's AST? That's the only thing I can think of that would make it so that the absence of an optional semicolon causes silent failures

[ljharb]

That’s very strange. (altho minifying a package never makes sense, ever - minification is something that should only ever be done by a top-level app)

I’ll look into it.

[RPGillespie6]

Thanks, let me know if you can't repro, but I was able to repro in a fresh project with the latest eslint and eslint-plugin-import

altho minifying a package never makes sense, ever - minification is something that should only ever be done by a top-level app

Not sure what you mean by this. In my case it's a TypeScript file that is compiled to a minifed JS esmodule, which is then distributed for use by legacy non-typescript esmodules. We use this plugin to ensure the imports using that esmodule are correct.

[ljharb]

Why would it need to be minified, though? Transpiling is fine, but minification is something else altogether.

[RPGillespie6]

Why would it need to be minified, though? Transpiling is fine, but minification is something else altogether.

So that devs don't attempt to modify it. It's a hint to the developer that it's a build artifact of upstream source, and that they should be modifying the upstream source.

This is a very standard and common practice for distributing libraries. For example: https://pixijs.download/v8.5.2/pixi.min.mjs

This is PixiJS as an esmodule. If I'm using this in my project, I would hope:

import { bogus } from "./pixi.min.mjs"

Would get flagged by eslint-plugin-import because that's not a valid symbol exported by pixijs

[michaelfaith]

This is a very standard and common practice for distributing libraries.

It's common for libraries that are generally consumed directly through html / browser scripts (via cdn downloads), since it reduces download size / time. But I would assert that it's an antipattern for libraries that are consumed through package managers and ultimately built into an app. It negatively impacts developer experience. Shit happens and people need to debug code, and if you're trying to figure out what a library is doing to investigate an issue, having it minified is a huge pain in the ass, with no real benefit.

[ljharb]

Why would a developer ever be modifying third-party code? If they are willing to do that in the first place, then they're already violating every best practice that exists, and no hint is going to stop them.

For distributing libraries as a drop-in script tag, yes, but that's archaic and obsolete and from the earlier part of the past decade. npm packages are never minified.

[ljharb]

Can you provide the minified content of bogus.js, so i can make a test case?

[RPGillespie6]

Not sure why you are getting so defensive, no offense was intended. Minified javascript is valid javascript, we aren't using npm to manage any front end dependencies, just a few dev tools like eslint and esbuild (to strip types out of TS, which we then load directly as an ESM). You would open an issue against a json parser that couldn't parse minified json, wouldn't you?

Can you provide the minified content of bogus.js, so i can make a test case?

function foo(){console.log("foo")}export{foo};

Edit:

Also note some minifiers also do obfuscation as well, so you could end with something like:

function a(){console.log('foo')}export{a as foo};

(though in my case, I'm not obfuscating, just removing whitespace)

[ljharb]

Indeed, it's a bug, as I indicated previously, but it's always important to try to stop bad practices whenever they appear, and minifying a published package is one such bad practice :-)

I'm also very confused; this isn't a published package it seems, but a relative file - why would you be linting build output of any kind? In your OP I would expect both test.js and bogus.js to be the unminified untranspiled raw output, since they're both in the project. (it's relevant because I'm trying to set up the reproduction test case)

[RPGillespie6]

In this case it's a legacy codebase that was created in 2016. When it was first created it was an AngularJS app. Then Google abruptly killed AngularJS, and in our wrath we swore off JS frameworks in favor of vanilla esmodules, vanilla web components, etc. ("frameworks are faddish, but vanilla is forever"). So new pages started just using vanilla esmodules like <script type="module" src="/js/account.mjs">

This seemed to work well and is very simple. No dependency on npm, no build steps, no file watchers, no bundlers. Write modular code, but just hit refresh to pick up any changes. Now of course, over time, we kept getting bitten by JS's lack of typing and so we wanted to start using TypeScript. However, there are a lot of .mjs files, so it wasn't feasible to convert them to TS all at once. So we are converting them to TS piecemeal as we have bandwidth. But that causes a problem. If we convert a shared module, such as utils.mjs, to TS, then the .mjs files that haven't been convert to TS yet can't use it.

So we introduced esbuild to convert TS to JS so that the .mjs files that haven't been converted yet can still use the module. However, now we have a situation where it's easy to accidentally make a change to the wrong source file (TS vs JS), and so we minify the JS so that it's clear that you should not modify it if you need to make a change.

Eventually, all the mjs files will be converted to TS and we will no longer need eslint-plugin-import because tsc will check all the imports instead. But right now we are in a migratory period where half the modules are TS and half aren't, but we still want to be sure that we haven't messed up any imports on the ESMs that are still JS.

We have not yet bitten the bullet on going all-in on adding npm to the critical path. Currently npm is not in the critical path at all. The .mjs files derived from the TS are committed to the repo. This is primarily to accommodate a few non-developers that occasionally clone the repo, check out a branch, start the webserver, and then perform QA.

I'm not claiming this system is perfect, but I think it's a valid design decision to avoid the baggage that comes with npm, since it introduces quite a bit of complexity, maintenance burden, and risk to the system.

[soryy708]

Hold on. Lets not mix tc39 concerns with eslint-plugin-import concerns.

Linter failing silently is a bad failure mode, and I believe we should at least provide a crash report.

Once we'll know what caused the fault, we can triage.

[soryy708]

To figure out where it crashes (to make it report it not silently) it'll help to have a way to reproduce this fault.

@RPGillespie6 Can you please share some additional information about the minified file?

1. Which minification algorithm/tool was used?
2. Which modules syntax?

My thinking is, if we take a large open source library, run it through minification, and try to import from it - maybe we'll be able to reproduce this?

[RPGillespie6]

1. I'm using esbuild --minify-whitespace, but I've also confirmed the issue happens with npx minify
2. I believe "ECMAScript modules": https://developer.mozilla.org/en-US/docs/Web/JavaScript/Guide/Modules

I'm importing the modules natively in-browser with <script type="module" ...

I also suspect this could happen to non-minified files as well, if the export statement is merged with the previous line in a way that breaks the current symbol parser

[soryy708]

I have prepared a reproduction repository: https://github.com/soryy708/eslint-plugin-import-issue-3107

Indeed it fails to error on import of non-existent/non-exported as reported in this issue.

---

Of note: We're using import / export syntax in .js files, but one would expect to see them in .mjs files.

minify doesn't support .mjs files. If I run it with a file that ends in this extension, it throws:

Error: File type "mjs" not supported.
    at minify (file:///C:/Users/USER/Desktop/issue-3107/node_modules/minify/lib/minify.js:46:15)
    at file:///C:/Users/USER/Desktop/issue-3107/node_modules/minify/bin/minify.js:72:43
    at Array.map (<anonymous>)
    at uglifyFiles (file:///C:/Users/USER/Desktop/issue-3107/node_modules/minify/bin/minify.js:72:29)
    at async minify (file:///C:/Users/USER/Desktop/issue-3107/node_modules/minify/bin/minify.js:52:5)
    at async file:///C:/Users/USER/Desktop/issue-3107/node_modules/minify/bin/minify.js:32:1

[soryy708]

In my reproduction repository, I debugged it down to:

• eslint-plugin-import/src/rules/named.js

  Lines 46 to 49 in f0727a6

  	const imports = ExportMapBuilder.get(node.source.value, context);
  	if (imports == null || imports.parseGoal === 'ambiguous') {
  	return;
  	}

imports is null.

What I don't know is:

• Why does ExportMapBuilder.get(node.source.value, context) return null?
• Why do we exit silently in that situation?

[soryy708]

ExportMapBuilder gives us a cached null.

The null was written to the cache in a previous visit, via:

• eslint-plugin-import/src/exportMap/builder.js

  Lines 76 to 81 in f0727a6

  	// check for and cache unambiguous modules
  	if (!unambiguous.test(content)) {
  	log('ignored path due to unambiguous regex:', path);
  	exportCache.set(cacheKey, null);
  	return null;
  	}

Which invokes:

• eslint-plugin-import/utils/unambiguous.js

  Lines 5 to 18 in f0727a6

  	const pattern = /(^|;)\s*(export|import)((\s+\w)|(\s*[{*=]))|import\(/m;
  	/**
  	* detect possible imports/exports without a full parse.
  	*
  	* A negative test means that a file is definitely _not_ a module.
  	* A positive test means it _could_ be.
  	*
  	* Not perfect, just a fast way to disqualify large non-ES6 modules and
  	* avoid a parse.
  	* @type {import('./unambiguous').test}
  	*/
  	exports.test = function isMaybeUnambiguousModule(content) {
  	return pattern.test(content);
  	};

[soryy708]

This failure is in fact not silent.

If we would have set the environment variable DEBUG=eslint-plugin-import:*, we would have seen:

eslint-plugin-import:resolver:node Resolving: ./bogus-min.js from: C:\Users\USER\Desktop\issue-3107\public\test.js +0ms
eslint-plugin-import:resolver:node Resolved to: C:\Users\USER\Desktop\issue-3107\public\bogus-min.js +2ms
eslint-plugin-import:ExportMap ignored path due to unambiguous regex: C:\Users\USER\Desktop\issue-3107\public\bogus-min.js +0ms
eslint-plugin-import:resolver:node Resolving: ./pixi.min.mjs from: C:\Users\USER\Desktop\issue-3107\public\test.js +5ms
eslint-plugin-import:resolver:node Resolved to: C:\Users\USER\Desktop\issue-3107\public\pixi.min.mjs +0ms
eslint-plugin-import:ExportMap ignored path due to unambiguous regex: C:\Users\USER\Desktop\issue-3107\public\pixi.min.mjs +4ms

[michaelfaith]

Looks like it's failing to match because the regex is looking for the word export or import with a trailing whitespace character. The minified file has an export, but immediately followed by {

[ljharb]

Indeed, seems like the regex could be tweaked to handle that case.

[RPGillespie6]

This part of the regex: (^|;)\s* implies that there must be either a semicolon or line start before export, which could also break in this case:

function y() {
    console.log("y");
}export {y};

Since } is neither a line start nor ;