From a8fd0cc55f0e001e8f9a21b6151970ee42c45d0b Mon Sep 17 00:00:00 2001
From: Emily Xiong <xiongemi@gmail.com>
Date: Wed, 5 Jun 2024 17:02:48 -0400
Subject: [PATCH] fix(core): add quotes around string to command (#23056)
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

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

<!-- This is the behavior we have today -->

- nx run will not have quotes, but underlying command will have

<!-- This is the behavior we should expect with the changes in this PR
-->
<img width="524" alt="Screenshot 2024-05-23 at 2 07 20 PM"
src="https://github.com/nrwl/nx/assets/16211801/7c96f884-3c11-4f56-b6b4-b3fd41ac2187">
<img width="471" alt="Screenshot 2024-05-23 at 2 07 03 PM"
src="https://github.com/nrwl/nx/assets/16211801/b6746a25-ebfc-4cb2-ad1d-4f8600782037">

<!-- Please link the issue being fixed so it gets closed when this is
merged. -->

Fixes #
---
 packages/rollup/src/plugins/plugin.spec.ts |  8 +++---
 packages/rollup/src/plugins/plugin.ts      | 33 ++++++++++++++++------
 2 files changed, 29 insertions(+), 12 deletions(-)

diff --git a/packages/rollup/src/plugins/plugin.spec.ts b/packages/rollup/src/plugins/plugin.spec.ts
index c8fcf7ec48638..8a974380e7c22 100644
--- a/packages/rollup/src/plugins/plugin.spec.ts
+++ b/packages/rollup/src/plugins/plugin.spec.ts
@@ -1,4 +1,4 @@
-import { type CreateNodesContext, joinPathFragments } from '@nx/devkit';
+import { type CreateNodesContext } from '@nx/devkit';
 import { createNodes } from './plugin';
 import { TempFs } from 'nx/src/internal-testing-utils/temp-fs';
 
@@ -10,9 +10,6 @@ jest.mock('rollup/loadConfigFile', () => {
   };
 });
 
-// @ts-ignore
-import { loadConfigFile } from 'rollup/loadConfigFile';
-
 describe('@nx/rollup/plugin', () => {
   let createNodesFunction = createNodes[1];
   let context: CreateNodesContext;
@@ -65,6 +62,7 @@ describe('@nx/rollup/plugin', () => {
       }`
       );
 
+      const { loadConfigFile } = require('rollup/loadConfigFile');
       loadConfigFile.mockReturnValue(rollupConfigOptions);
 
       process.chdir(tempFs.tempDir);
@@ -140,6 +138,8 @@ describe('@nx/rollup/plugin', () => {
       console.log("hello world");
       }`
       );
+
+      const { loadConfigFile } = require('rollup/loadConfigFile');
       loadConfigFile.mockReturnValue(rollupConfigOptions);
 
       process.chdir(tempFs.tempDir);
diff --git a/packages/rollup/src/plugins/plugin.ts b/packages/rollup/src/plugins/plugin.ts
index a76f4644d944e..d46690c8d5c79 100644
--- a/packages/rollup/src/plugins/plugin.ts
+++ b/packages/rollup/src/plugins/plugin.ts
@@ -2,24 +2,20 @@ import { workspaceDataDirectory } from 'nx/src/utils/cache-directory';
 import { basename, dirname, join } from 'path';
 import { existsSync, readdirSync } from 'fs';
 import {
-  type TargetConfiguration,
   type CreateDependencies,
   type CreateNodes,
-  readJsonFile,
-  writeJsonFile,
-  detectPackageManager,
   CreateNodesContext,
+  detectPackageManager,
   joinPathFragments,
+  readJsonFile,
+  type TargetConfiguration,
+  writeJsonFile,
 } from '@nx/devkit';
 import { calculateHashForCreateNodes } from '@nx/devkit/src/utils/calculate-hash-for-create-nodes';
 import { getLockFileName } from '@nx/js';
 import { getNamedInputs } from '@nx/devkit/src/utils/get-named-inputs';
 import { type RollupOptions } from 'rollup';
 
-// This import causes an error due to the module resolution used. If we switch to bundler or nodenext in the future we remove this ignore.
-// @ts-ignore
-import { loadConfigFile } from 'rollup/loadConfigFile';
-
 const cachePath = join(workspaceDataDirectory, 'rollup.hash');
 const targetsCache = readTargetsCache();
 
@@ -94,6 +90,27 @@ async function buildRollupTarget(
   options: RollupPluginOptions,
   context: CreateNodesContext
 ): Promise<Record<string, TargetConfiguration>> {
+  let loadConfigFile: (
+    path: string,
+    commandOptions: unknown,
+    watchMode: boolean
+  ) => Promise<{ options: RollupOptions[] }>;
+
+  try {
+    // Try to load the workspace version of rollup first (it should already exist).
+    // Using the workspace rollup ensures that the config file is compatible with the `loadConfigFile` function.
+    // e.g. rollup@2 supports having `require` calls in rollup config, but rollup@4 does not.
+    const m = require(require.resolve('rollup/loadConfigFile', {
+      paths: [dirname(configFilePath)],
+    }));
+    // Rollup 2 has this has default export, but it is named in 3 and 4.
+    // See: https://www.unpkg.com/browse/rollup@2.79.1/dist/loadConfigFile.js
+    loadConfigFile = typeof m === 'function' ? m : m.loadConfigFile;
+  } catch {
+    // Fallback to our own if needed.
+    loadConfigFile = require('rollup/loadConfigFile').loadConfigFile;
+  }
+
   const namedInputs = getNamedInputs(projectRoot, context);
   const rollupConfig = (
     (await loadConfigFile(