robinloeffel / esbuild-plugin-eslint Goto Github PK

After bumping esbuild-plugin-eslint from v0.3.3 to v0.3.8 in order to use the new release from #11, I encountered the problem of importing ESM in a CJS environment (which appears to have been introduced in v0.3.4). Since this is for use with the Serverless Framework (using serverless-esbuild), I can't just move to ESM, unfortunately.

To work around this, I added https://github.com/nktnet1/import-sync in order to import this plugin. This worked great, as I got back to the error of "no eslint configuration found" from v0.3.3. However, once I bumped eslint to the v9 beta, I got the following error

✘ [ERROR] A dynamic import callback was not specified. [plugin eslint]

    /home/james/projects/LabelLogicLive_Monorepo/packages/api/node_modules/eslint/lib/eslint/eslint.js:315:19:
      315 │     const config = (await import(fileURL)).default;
          ╵                    ^

    at new NodeError (node:internal/errors:393:5)
    at importModuleDynamicallyCallback (node:internal/process/esm_loader:39:9)
    at loadFlatConfigFile (/home/james/projects/LabelLogicLive_Monorepo/packages/api/node_modules/eslint/lib/eslint/eslint.js:315:20)
    at async calculateConfigArray (/home/james/projects/LabelLogicLive_Monorepo/packages/api/node_modules/eslint/lib/eslint/eslint.js:394:28)
    at async ESLint.lintFiles (/home/james/projects/LabelLogicLive_Monorepo/packages/api/node_modules/eslint/lib/eslint/eslint.js:803:25)
    at async /home/james/projects/LabelLogicLive_Monorepo/packages/api/node_modules/esbuild-plugin-eslint/dist/index.js:15:29
    at async /home/james/projects/LabelLogicLive_Monorepo/packages/api/node_modules/esbuild/lib/main.js:1481:27

  This error came from the "onEnd" callback registered here:

    /home/james/projects/LabelLogicLive_Monorepo/packages/api/node_modules/esbuild-plugin-eslint/dist/index.js:14:8:
      14 │         onEnd(async () => {
         ╵         ~~~~~

This makes me wonder whether the problem is with the importing of ESM in a CJS environment, since my workaround I had while waiting on your fix for #11 was working perfectly fine without the need for any hacks. I also tried it with v9 of eslint, and it still worked perfectly fine. The problem is somewhere between importing esbuild-plugin-eslint, and the loading of eslint itself.

const eslintPlugin = ({ filter }) => ({
    name: "eslint",
    setup: ({ onLoad, onEnd }) => {
        loadESLint().then(eslintClass => {
	    /** @type {import("esbuild").OnLoadArgs["path"][]} */
	    const filesToLint = [];
	    const eslint = new eslintClass();

	    onLoad({ filter }, ({ path }) => {
	        if (!path.includes("node_modules")) filesToLint.push(path);
	        return null;
	    });

	    onEnd(async () => {
		const results = await eslint.lintFiles(filesToLint);
		const formatter = await eslint.loadFormatter("stylish");
		const output = await formatter.format(results);
		if (output.length > 0) console.log(output);

		const warnings = results.reduce(
		    (count, result) => count + result.warningCount,
		    0
	    	);
	        const errors = results.reduce(
		    (count, result) => count + result.errorCount,
		    0
		);

		const ret = { errors: [] };
		if (warnings > 0) {
		    ret.errors.push({
			text: `${warnings} warnings were found by eslint!`
		    });
		}

                if (errors > 0) {
                    ret.errors.push(
			{ text: `${errors} errors were found by eslint!` }
		    );
                }
                return ret;
            });
        });
    }
});

Would it be possible to get a release of esbuild-plugin-eslint with a cjs build alongside the esm build?

For example, the dist folder would look like

dist
|- cjs
| |- index.cjs
|- esm
| |- index.mjs
|- types
  |- index.d.ts

and the package.json would include the following

"exports": {
  "require": "dist/cjs/index.cjs",
  "import": "dist/esm/index.mjs",
  "types": "dist/types/index.d.ts"
}

Otherwise projects can build with esbuild 17 which is the latest released.

The problematic line is 9b25f90#diff-88a99604221821c7258fe9c24fc58a4ef42242b97aeae97d6c438db3f62be6cdR23

Eslint will return an array of results, one result per file. With this change you store in the 'result' variable the first item of the array.
See:
const [result] = [1,2,3,4]
result will be here 1

This is not what you want, as in this case, it will ignore all errors apart from the ones for the first file.

esbuild-plugin-esling makes eslint process node_modules, which can get annoying if some modules depends on internal eslint linter plugins (like react-dom) or doesn't follow defined codestyle.
ignoring files with eslint makes ignoring messages which are annoyingly hard to remove.

Pull request fixes #1 by adding a glob patterns config to filter files to process before eslint call for each file.

I'm looking for a way to create eslint bundle, i.e one file eslint.js with all necessary dependencies from node_modules bundled in to that file. because of dynamic imports, out of the box it does not work:

$ esbuild --platform=node --target=node14 --bundle --sourcemap --outfile=dist/eslint.js node_modules/.bin/eslint

  dist/eslint.js      3.3mb ⚠️
  dist/eslint.js.map  6.4mb

✨  Done in 0.32s.
$ node dist/eslint.js
There was a problem loading formatter: /Users/glen/work/dist/formatters/stylish
Error: Cannot find module '/Users/glen/work/dist/formatters/stylish'
Require stack:
- /Users/glen/work/dist/eslint.js

I setup your plugin into build() pipeline, and it still gave me formatters/stylish import error.

Similarly for bundling pino, there's plugin that handles the dynamic imports for bundling:

• https://www.npmjs.com/package/esbuild-plugin-pino

but this project, given the same project name structure, and usage, does something different. what does it mean? because these two sentences definitely doesn't explain it. to me:

Lint your esbuild bundles with eslint. 🧐

Nicely integrates the most recent version of eslint into an esbuild plugin.

please describe the actual usage of this plugin in readme, perhaps with examples, asciicast?

If ESLint warnings or errors are encountered, they aren't reported back to esbuild's build result. That means from esbuild's perspective, the build still succeeded.

As a workaround, we manually grep through the esbuild output for ESLint errors, because we want the build to fail if there are any ESLint errors. It's possible that we'd want this behavior configurable--e.g. fail in production builds, but succeed in dev builds. However, since the plugin uses onEnd it's unclear if the bundles would still be written even if there are errors (desired behavior for dev builds, to be able to quickly write some noncompliant debugging code and try it out).

The esbuild docs make this sound relatively straightforward, but I'd consider submitting a pull request for this behavior.

https://github.com/robinloeffel/esbuild-plugin-eslint/blob/main/source/index.ts#L49

In this line you need to return the 'errros' attribute so that esbuild will throw an exception. Additionally the throwOnErrors should be 'true' by default I feel.

ESLint 8.57 was released 6 days ago, and includes a migration helper in the form of loadESLint().

Given that ESLint v9 will require the use of the flat config format by default, I've been working on migrating my eslint config to the flat config format. However, this plugin is the last step in that migration.

In the meantime, I've copied the plugin into my codebase and modified it to use the loadESLint function. I'd prefer not to have this plugin code in my esbuild config file, since it doubles the size of the file alone.