fix(bundling): register plugin correctly in rollup migration generator (#26673)

<!-- 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 -->

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

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

Fixes #

packages/rollup/package.json

@@ -37,6 +37,7 @@
     "autoprefixer": "^10.4.9",
     "babel-plugin-transform-async-to-promises": "^0.8.15",
     "chalk": "^4.1.0",
+    "minimatch": "9.0.3",
     "postcss": "^8.4.38",
     "rollup": "^4.14.0",
     "rollup-plugin-copy": "^3.5.0",

packages/rollup/src/generators/convert-to-inferred/convert-to-inferred.spec.ts

@@ -1,12 +1,15 @@
-import { createTreeWithEmptyWorkspace } from '@nx/devkit/testing';
 import {
   addProjectConfiguration,
-  type ProjectConfiguration,
   readNxJson,
   readProjectConfiguration,
-  type Tree,
   updateNxJson,
+  updateProjectConfiguration,
+  type ExpandedPluginConfiguration,
+  type ProjectConfiguration,
+  type Tree,
 } from '@nx/devkit';
+import { createTreeWithEmptyWorkspace } from '@nx/devkit/testing';
+import type { RollupPluginOptions } from '../../plugins/plugin';
 import convertToInferred from './convert-to-inferred';
 
 interface CreateProjectOptions {
@@ -91,9 +94,10 @@ describe('Rollup - Convert Executors To Plugin', () => {
       expect(readNxJson(tree).plugins).toEqual([
         {
           options: {
-            targetName: 'build',
+            buildTargetName: 'build',
           },
           plugin: '@nx/rollup/plugin',
+          include: [`${project.root}/**/*`],
         },
       ]);
       expect(tree.read('mypkg/rollup.config.js', 'utf-8'))
@@ -125,6 +129,38 @@ describe('Rollup - Convert Executors To Plugin', () => {
       expect(readProjectConfiguration(tree, project.name).targets).toEqual({});
     });
 
+    it('should remove "includes" from the plugin registration when all projects are included', async () => {
+      const project = createProject(tree, {
+        name: 'mypkg',
+        root: 'mypkg',
+      });
+      const project2 = createProject(tree, {
+        name: 'otherpkg1',
+        root: 'otherpkg1',
+      });
+      project2.targets = {};
+      updateProjectConfiguration(tree, project2.name, project2);
+      const nxJson = readNxJson(tree);
+      nxJson.plugins ??= [];
+      nxJson.plugins.push({
+        plugin: '@nx/rollup/plugin',
+        options: { buildTargetName: 'build' },
+        include: [`${project2.root}/**/*`],
+      });
+      updateNxJson(tree, nxJson);
+
+      await convertToInferred(tree, { project: project.name });
+
+      // nx.json modifications
+      const nxJsonPlugins = readNxJson(tree).plugins;
+      const rollupPluginRegistrations = nxJsonPlugins.filter(
+        (plugin): plugin is ExpandedPluginConfiguration<RollupPluginOptions> =>
+          typeof plugin !== 'string' && plugin.plugin === '@nx/rollup/plugin'
+      );
+      expect(rollupPluginRegistrations.length).toBe(1);
+      expect(rollupPluginRegistrations[0].include).toBeUndefined();
+    });
+
     it('should support existing rollupConfig files', async () => {
       const projectWithSingleConfig = createProject(tree, {
         name: 'mypkg1',
@@ -533,7 +569,7 @@ describe('Rollup - Convert Executors To Plugin', () => {
       expect(readNxJson(tree).plugins).toEqual([
         {
           options: {
-            targetName: 'build',
+            buildTargetName: 'build',
           },
           plugin: '@nx/rollup/plugin',
         },
@@ -579,6 +615,11 @@ describe('Rollup - Convert Executors To Plugin', () => {
         name: 'pkg2',
         root: 'pkg2',
       });
+      createProject(tree, {
+        name: 'pkg3',
+        root: 'pkg3',
+        targetName: 'build-rollup',
+      });
 
       await convertToInferred(tree, {});
 
@@ -632,6 +673,24 @@ describe('Rollup - Convert Executors To Plugin', () => {
       `);
       expect(readProjectConfiguration(tree, 'pkg1').targets).toEqual({});
       expect(readProjectConfiguration(tree, 'pkg2').targets).toEqual({});
+      // nx.json modifications
+      const nxJsonPlugins = readNxJson(tree).plugins;
+      const rollupPluginRegistrations = nxJsonPlugins.filter(
+        (plugin): plugin is ExpandedPluginConfiguration<RollupPluginOptions> =>
+          typeof plugin !== 'string' && plugin.plugin === '@nx/rollup/plugin'
+      );
+      expect(rollupPluginRegistrations.length).toBe(2);
+      expect(rollupPluginRegistrations[0].options.buildTargetName).toBe(
+        'build'
+      );
+      expect(rollupPluginRegistrations[0].include).toStrictEqual([
+        `pkg1/**/*`,
+        `pkg2/**/*`,
+      ]);
+      expect(rollupPluginRegistrations[1].options.buildTargetName).toBe(
+        'build-rollup'
+      );
+      expect(rollupPluginRegistrations[1].include).toStrictEqual([`pkg3/**/*`]);
     });
   });
 });

packages/rollup/src/generators/convert-to-inferred/convert-to-inferred.ts

@@ -2,14 +2,14 @@ import {
   formatFiles,
   getProjects,
   readNxJson,
-  type Tree,
-  updateNxJson,
   updateProjectConfiguration,
+  type Tree,
 } from '@nx/devkit';
 import { forEachExecutorOptions } from '@nx/devkit/src/generators/executor-options-utils';
-
+import type { RollupExecutorOptions } from '../../executors/rollup/schema';
+import type { RollupPluginOptions } from '../../plugins/plugin';
 import { extractRollupConfigFromExecutorOptions } from './lib/extract-rollup-config-from-executor-options';
-import { RollupExecutorOptions } from '../../executors/rollup/schema';
+import { addPluginRegistrations } from './lib/add-plugin-registrations';
 
 interface Schema {
   project?: string;
@@ -17,9 +17,8 @@ interface Schema {
 }
 
 export async function convertToInferred(tree: Tree, options: Schema) {
-  let migrated = 0;
+  const migratedProjects = new Map<string, RollupPluginOptions>();
-
+  let projects = getProjects(tree);
-  const projects = getProjects(tree);
 
   forEachExecutorOptions<RollupExecutorOptions>(
     tree,
@@ -96,31 +95,22 @@ export async function convertToInferred(tree: Tree, options: Schema) {
 
       updateProjectConfiguration(tree, projectName, project);
 
-      nxJson.plugins ??= [];
+      migratedProjects.set(projectName, { buildTargetName: targetName });
-      if (
-        !nxJson.plugins.some((p) =>
-          typeof p === 'string'
-            ? p === '@nx/rollup/plugin'
-            : p.plugin === '@nx/rollup/plugin'
-        )
-      ) {
-        nxJson.plugins.push({
-          plugin: '@nx/rollup/plugin',
-          options: {
-            targetName: 'build',
-          },
-        });
-      }
-      updateNxJson(tree, nxJson);
-
-      migrated++;
     }
   );
 
-  if (migrated === 0) {
+  if (migratedProjects.size === 0) {
     throw new Error('Could not find any targets to migrate.');
   }
 
+  projects = getProjects(tree);
+  await addPluginRegistrations<RollupPluginOptions>(
+    tree,
+    migratedProjects,
+    projects,
+    '@nx/rollup/plugin'
+  );
+
   if (!options.skipFormat) {
     await formatFiles(tree);
   }

packages/rollup/src/generators/convert-to-inferred/lib/add-plugin-registrations.ts

@@ -0,0 +1,74 @@
+import {
+  readNxJson,
+  updateNxJson,
+  type ExpandedPluginConfiguration,
+  type ProjectConfiguration,
+  type Tree,
+} from '@nx/devkit';
+import { minimatch } from 'minimatch';
+
+export async function addPluginRegistrations<T>(
+  tree: Tree,
+  projectTargets: Map<string, T>,
+  projects: Map<string, ProjectConfiguration>,
+  pluginPath: string
+): Promise<void> {
+  const nxJson = readNxJson(tree);
+
+  for (const [project, options] of projectTargets.entries()) {
+    const existingPlugin = nxJson.plugins?.find(
+      (plugin): plugin is ExpandedPluginConfiguration =>
+        typeof plugin !== 'string' &&
+        plugin.plugin === pluginPath &&
+        Object.keys(options).every(
+          (key) => plugin.options[key] === options[key]
+        )
+    );
+
+    const projectIncludeGlob = `${projects.get(project).root}/**/*`;
+    if (!existingPlugin) {
+      nxJson.plugins ??= [];
+      const plugin: ExpandedPluginConfiguration = {
+        plugin: pluginPath,
+        options,
+        include: [projectIncludeGlob],
+      };
+
+      nxJson.plugins.push(plugin);
+    } else if (existingPlugin.include) {
+      if (
+        !existingPlugin.include.some((include) =>
+          minimatch(projectIncludeGlob, include, { dot: true })
+        )
+      ) {
+        existingPlugin.include.push(projectIncludeGlob);
+      }
+    }
+  }
+
+  if (!areProjectsUsingTheExecutorLeft(projects)) {
+    // all projects have been migrated, if there's only one plugin registration
+    // left, remove its "include" property
+    const pluginRegistrations = nxJson.plugins?.filter(
+      (plugin): plugin is ExpandedPluginConfiguration =>
+        typeof plugin !== 'string' && plugin.plugin === pluginPath
+    );
+    if (pluginRegistrations?.length === 1) {
+      for (const plugin of pluginRegistrations) {
+        delete plugin.include;
+      }
+    }
+  }
+
+  updateNxJson(tree, nxJson);
+}
+
+function areProjectsUsingTheExecutorLeft(
+  projects: Map<string, ProjectConfiguration>
+): boolean {
+  return Array.from(projects.values()).some((project) =>
+    Object.values(project.targets ?? {}).some(
+      (target) => target.executor === '@nx/rollup:rollup'
+    )
+  );
+}