Skip to content

Commit

Permalink
fix(linter): convert root projects correctly to inferred and remove d…
Browse files Browse the repository at this point in the history
…efault option values (#27035)

<!-- Please make sure you have read the submission guidelines before
posting an PR -->
<!--
https://github.com/nrwl/nx/blob/master/CONTRIBUTING.md#-submitting-a-pr
-->

<!-- Please make sure that your commit message follows our format -->
<!-- Example: `fix(nx): must begin with lowercase` -->

<!-- If this is a particularly complex change or feature addition, you
can request a dedicated Nx release for this pull request branch. Mention
someone from the Nx team or the `@nrwl/nx-pipelines-reviewers` and they
will confirm if the PR warrants its own release for testing purposes,
and generate it for you if appropriate. -->

## Current Behavior
<!-- This is the behavior we have today -->

Converting a root project to inferred results in the plugin registration
to have the `includes` option set to `./**/*`. This is invalid and
causes no project to be inferred.

Additionally, the eslint `convert-to-inferred` generator:

- keeps a redundant `config` option, which is not needed because
inferred tasks only work with default/known ESLint config files, so
there's no need to specify it in the options
- converts all `lintFilePatterns` to `args`, which is correct, but it
keeps the patterns that are already inferred by the plugin, which leads
to duplicated patterns when running the task

## Expected Behavior
<!-- This is the behavior we should expect with the changes in this PR
-->

Converting a root project to inferred should work as expected and result
in the `lint` task being inferred for the project.
Also, default inferred options should be removed from the target
options.

## Related Issue(s)
<!-- Please link the issue being fixed so it gets closed when this is
merged. -->

Fixes #26775
  • Loading branch information
leosvelperez authored Jul 22, 2024
1 parent 0bc8898 commit 5217c33
Show file tree
Hide file tree
Showing 3 changed files with 315 additions and 49 deletions.
Original file line number Diff line number Diff line change
@@ -1,5 +1,6 @@
import { minimatch } from 'minimatch';
import { deepStrictEqual } from 'node:assert';
import { join } from 'node:path/posix';
import type {
InputDefinition,
ProjectConfiguration,
Expand Down Expand Up @@ -39,7 +40,8 @@ type PostTargetTransformer = (
inferredTargetConfiguration: InferredTargetConfiguration
) => TargetConfiguration | Promise<TargetConfiguration>;
type SkipTargetFilter = (
targetConfiguration: TargetConfiguration
targetOptions: Record<string, unknown>,
projectConfiguration: ProjectConfiguration
) => false | string;
type SkipProjectFilter = (
projectConfiguration: ProjectConfiguration
Expand All @@ -58,7 +60,6 @@ class ExecutorToPluginMigrator<T> {
#nxJson: NxJsonConfiguration;
#targetDefaultsForExecutor: Partial<TargetConfiguration>;
#targetAndProjectsToMigrate: Map<string, Set<string>>;
#pluginToAddForTarget: Map<string, ExpandedPluginConfiguration<T>>;
#createNodes?: CreateNodes<T>;
#createNodesV2?: CreateNodesV2<T>;
#createNodesResultsForTargets: Map<string, ConfigurationResult>;
Expand Down Expand Up @@ -108,7 +109,6 @@ class ExecutorToPluginMigrator<T> {
nxJson.plugins ??= [];
this.#nxJson = nxJson;
this.#targetAndProjectsToMigrate = new Map();
this.#pluginToAddForTarget = new Map();
this.#createNodesResultsForTargets = new Map();
this.#skippedProjects = new Set();

Expand All @@ -118,18 +118,11 @@ class ExecutorToPluginMigrator<T> {
}

async #migrateTarget(targetName: string) {
const include: string[] = [];
for (const projectName of this.#targetAndProjectsToMigrate.get(
targetName
)) {
include.push(await this.#migrateProject(projectName, targetName));
await this.#migrateProject(projectName, targetName);
}

this.#pluginToAddForTarget.set(targetName, {
plugin: this.#pluginPath,
options: this.#pluginOptionsBuilder(targetName),
include,
});
}

async #migrateProject(projectName: string, targetName: string) {
Expand Down Expand Up @@ -180,8 +173,6 @@ class ExecutorToPluginMigrator<T> {
}

updateProjectConfiguration(this.tree, projectName, projectConfig);

return `${projectFromGraph.data.root}/**/*`;
}

#mergeInputs(
Expand Down Expand Up @@ -237,7 +228,12 @@ class ExecutorToPluginMigrator<T> {
forEachExecutorOptions(
this.tree,
this.#executor,
(targetConfiguration, projectName, targetName, configurationName) => {
(
options: Record<string, unknown>,
projectName,
targetName,
configurationName
) => {
if (this.#skippedProjects.has(projectName) || configurationName) {
return;
}
Expand All @@ -263,9 +259,12 @@ class ExecutorToPluginMigrator<T> {
return;
}

const skipTargetReason = this.#skipTargetFilter(targetConfiguration);
const skipTargetReason = this.#skipTargetFilter(
options,
this.#projectGraph.nodes[projectName].data
);
if (skipTargetReason) {
const errorMsg = `${targetName} target on project "${projectName}" cannot be migrated. ${skipTargetReason}`;
const errorMsg = `The ${targetName} target on project "${projectName}" cannot be migrated. ${skipTargetReason}`;
if (this.#specificProjectToMigrate) {
throw new Error(errorMsg);
} else {
Expand Down Expand Up @@ -538,7 +537,10 @@ async function addPluginRegistrations<T>(
)
);

const projectIncludeGlob = `${projectGraph.nodes[project].data.root}/**/*`;
const projectIncludeGlob =
projectGraph.nodes[project].data.root === '.'
? '*'
: join(projectGraph.nodes[project].data.root, '**/*');
if (!existingPlugin) {
nxJson.plugins ??= [];
const plugin: ExpandedPluginConfiguration = {
Expand Down
Loading

0 comments on commit 5217c33

Please sign in to comment.