Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

fix: Use original plugin from disk in FlatCompat #137

Merged
merged 3 commits into from
Nov 27, 2023
Merged

fix: Use original plugin from disk in FlatCompat #137

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
e109c00
fix: Use original plugin from disk in FlatCompat
nzakas Nov 22, 2023
9450dce
Clean up lint errors
nzakas Nov 22, 2023
d3d57e4
Hide original field from serialization
nzakas Nov 27, 2023
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 2 additions & 0 deletions lib/config-array-factory.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1053,6 +1053,7 @@ class ConfigArrayFactory {
if (plugin) {
return new ConfigDependency({
definition: normalizePlugin(plugin),
original: plugin,
filePath: "", // It's unknown where the plugin came from.
id,
importerName: ctx.name,
Expand Down Expand Up @@ -1089,6 +1090,7 @@ class ConfigArrayFactory {

return new ConfigDependency({
definition: normalizePlugin(pluginDefinition),
original: pluginDefinition,
filePath,
id,
importerName: ctx.name,
Expand Down
11 changes: 10 additions & 1 deletion lib/config-array/config-dependency.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -28,6 +28,7 @@ class ConfigDependency {
* Initialize this instance.
* @param {Object} data The dependency data.
* @param {T} [data.definition] The dependency if the loading succeeded.
* @param {T} [data.original] The original, non-normalized dependency if the loading succeeded.
* @param {Error} [data.error] The error object if the loading failed.
* @param {string} [data.filePath] The actual path to the dependency if the loading succeeded.
* @param {string} data.id The ID of this dependency.
Expand All @@ -36,6 +37,7 @@ class ConfigDependency {
*/
constructor({
definition = null,
original = null,
error = null,
filePath = null,
id,
Expand All @@ -49,6 +51,12 @@ class ConfigDependency {
*/
this.definition = definition;

/**
* The original dependency as loaded directly from disk if the loading succeeded.
* @type {T|null}
*/
this.original = original;

/**
* The error object if the loading failed.
* @type {Error|null}
Expand Down Expand Up @@ -101,7 +109,8 @@ class ConfigDependency {
*/
[util.inspect.custom]() {
const {
definition: _ignore, // eslint-disable-line no-unused-vars
definition: _ignore1, // eslint-disable-line no-unused-vars
original: _ignore2, // eslint-disable-line no-unused-vars
Comment on lines +112 to +113
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Seems like https://eslint.org/docs/latest/rules/no-unused-vars#varsignorepattern could be useful here, and then you can do something like

Suggested change
definition: _ignore1, // eslint-disable-line no-unused-vars
original: _ignore2, // eslint-disable-line no-unused-vars
definition: _,
original: __,

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Or https://eslint.org/docs/latest/rules/no-unused-vars#ignorerestsiblings

...obj
} = this;

Expand Down
2 changes: 1 addition & 1 deletion lib/flat-compat.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -132,7 +132,7 @@ function translateESLintRC(eslintrcConfig, {
debug(`Translating plugin: ${pluginName}`);
debug(`Resolving plugin '${pluginName} relative to ${resolvePluginsRelativeTo}`);

const { definition: plugin, error } = eslintrcConfig.plugins[pluginName];
const { original: plugin, error } = eslintrcConfig.plugins[pluginName];

if (error) {
throw error;
Expand Down
4 changes: 4 additions & 0 deletions tests/lib/config-array-factory.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -631,6 +631,10 @@ describe("ConfigArrayFactory", () => {
it("should have the path to the package at 'plugins[id].filePath' property.", () => {
assert.strictEqual(element.plugins.xxx.filePath, path.join(getPath(), "node_modules/custom-eslint-plugin-xxx/index.js"));
});

it("should have the original definition equal to the origina plugin object", () => {
assert.strictEqual(element.plugins.xxx.original, require(path.join(getPath(), "node_modules/custom-eslint-plugin-xxx/index.js")));
});
});

describe("if 'extends' property was 'foo', the returned value", () => {
Expand Down
63 changes: 16 additions & 47 deletions tests/lib/flat-compat.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -12,31 +12,17 @@ import { fileURLToPath, pathToFileURL } from "url";
import { assert } from "chai";
import { FlatCompat } from "../../lib/index.js";
import environments from "../../conf/environments.js";
import { createRequire } from "module";

const dirname = path.dirname(fileURLToPath(import.meta.url));
const require = createRequire(import.meta.url);

//-----------------------------------------------------------------------------
// Helpers
//-----------------------------------------------------------------------------

const FIXTURES_BASE_PATH = path.resolve(dirname, "../fixtures/flat-compat/");

/**
* Normalizes a plugin object to have all available keys. This matches what
* ConfigArrayFactory does.
* @param {Object} plugin The plugin object to normalize.
* @returns {Object} The normalized plugin object.
*/
function normalizePlugin(plugin) {
return {
configs: {},
rules: {},
environments: {},
processors: {},
...plugin
};
}

/**
* Returns the full directory path for a fixture directory.
* @param {string} dirName The directory name to resolve.
Expand All @@ -56,9 +42,9 @@ describe("FlatCompat", () => {

let compat;
const baseDirectory = getFixturePath("config");
const pluginFixture1 = normalizePlugin((await import(pathToFileURL(path.join(baseDirectory, "node_modules/eslint-plugin-fixture1.js")))).default);
const pluginFixture2 = normalizePlugin((await import(pathToFileURL(path.join(baseDirectory, "node_modules/eslint-plugin-fixture2.js")))).default);
const pluginFixture3 = normalizePlugin((await import(pathToFileURL(path.join(baseDirectory, "node_modules/eslint-plugin-fixture3.js")))).default);
const pluginFixture1 = (await import(pathToFileURL(path.join(baseDirectory, "node_modules/eslint-plugin-fixture1.js")))).default;
const pluginFixture2 = (await import(pathToFileURL(path.join(baseDirectory, "node_modules/eslint-plugin-fixture2.js")))).default;
const pluginFixture3 = (await import(pathToFileURL(path.join(baseDirectory, "node_modules/eslint-plugin-fixture3.js")))).default;

beforeEach(() => {
compat = new FlatCompat({
Expand Down Expand Up @@ -1059,13 +1045,7 @@ describe("FlatCompat", () => {
assert.strictEqual(result.length, 1);
assert.deepStrictEqual(result[0], {
plugins: {
fixture1: {
configs: {},
rules: {},
environments: {},
processors: {},
...(await import(pathToFileURL(path.join(compat.baseDirectory, "node_modules/eslint-plugin-fixture1.js")))).default
}
fixture1: (await import(pathToFileURL(path.join(compat.baseDirectory, "node_modules/eslint-plugin-fixture1.js")))).default
}
});
});
Expand All @@ -1087,13 +1067,7 @@ describe("FlatCompat", () => {
});
assert.deepStrictEqual(result[1], {
plugins: {
fixture2: {
configs: {},
rules: {},
environments: {},
processors: {},
...plugin
}
fixture2: plugin
}
});
});
Expand All @@ -1109,24 +1083,19 @@ describe("FlatCompat", () => {
});
assert.deepStrictEqual(result[1], {
plugins: {
fixture1: {
configs: {},
rules: {},
environments: {},
processors: {},
...(await import(pathToFileURL(path.join(compat.baseDirectory, "node_modules/eslint-plugin-fixture1.js")))).default
},
fixture2: {
configs: {},
rules: {},
environments: {},
processors: {},
...plugin
}
fixture1: (await import(pathToFileURL(path.join(compat.baseDirectory, "node_modules/eslint-plugin-fixture1.js")))).default,
fixture2: plugin
}
});
});

it("should use the same plugin instance as require()", async () => {
const result = compat.config({ plugins: ["fixture2"] });
const plugin = require(path.join(compat.baseDirectory, "node_modules/eslint-plugin-fixture2.js"));

assert.strictEqual(result[1].plugins.fixture2, plugin);
});

});

});