import-js / eslint-plugin-import

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

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

Open RPGillespie6 opened 1 day ago

RPGillespie6 commented 1 day ago

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 commented 1 day ago

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 commented 18 hours ago

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 commented 17 hours ago

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

RPGillespie6 commented 17 hours ago

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 commented 16 hours ago

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 commented 15 hours ago

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 commented 15 hours ago

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

RPGillespie6 commented 10 hours ago

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)