Skip to content

Commit

Permalink
fix(core): add quotes around string to command (#23056)
Browse files Browse the repository at this point in the history
<!-- 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 #
  • Loading branch information
xiongemi authored and jaysoo committed Jun 6, 2024
1 parent bccb2c5 commit a8fd0cc
Show file tree
Hide file tree
Showing 2 changed files with 29 additions and 12 deletions.
8 changes: 4 additions & 4 deletions packages/rollup/src/plugins/plugin.spec.ts
Original file line number Diff line number Diff line change
@@ -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';

Expand All @@ -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;
Expand Down Expand Up @@ -65,6 +62,7 @@ describe('@nx/rollup/plugin', () => {
}`
);

const { loadConfigFile } = require('rollup/loadConfigFile');
loadConfigFile.mockReturnValue(rollupConfigOptions);

process.chdir(tempFs.tempDir);
Expand Down Expand Up @@ -140,6 +138,8 @@ describe('@nx/rollup/plugin', () => {
console.log("hello world");
}`
);

const { loadConfigFile } = require('rollup/loadConfigFile');
loadConfigFile.mockReturnValue(rollupConfigOptions);

process.chdir(tempFs.tempDir);
Expand Down
33 changes: 25 additions & 8 deletions packages/rollup/src/plugins/plugin.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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();

Expand Down Expand Up @@ -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/[email protected]/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(
Expand Down

0 comments on commit a8fd0cc

Please sign in to comment.