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.

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

fix(misc): ensure plugins are not creating workspace context while creating nodes #22967

Open
wants to merge 1 commit into
base: master
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
8 changes: 2 additions & 6 deletions packages/cypress/src/plugins/plugin.ts
Expand Up @@ -17,7 +17,7 @@ import { getLockFileName } from '@nx/js';

import { getNamedInputs } from '@nx/devkit/src/utils/get-named-inputs';
import { existsSync, readdirSync } from 'fs';
import { globWithWorkspaceContext } from 'nx/src/utils/workspace-context';
import { globAsync } from 'nx/src/utils/globs';
import { calculateHashForCreateNodes } from '@nx/devkit/src/utils/calculate-hash-for-create-nodes';
import { projectGraphCacheDirectory } from 'nx/src/utils/cache-directory';
import { NX_PLUGIN_OPTIONS } from '../utils/constants';
Expand Down Expand Up @@ -206,11 +206,7 @@ async function buildCypressTargets(
: Array.isArray(cypressConfig.e2e.excludeSpecPattern)
? cypressConfig.e2e.excludeSpecPattern.map((p) => join(projectRoot, p))
: [join(projectRoot, cypressConfig.e2e.excludeSpecPattern)];
const specFiles = globWithWorkspaceContext(
context.workspaceRoot,
specPatterns,
excludeSpecPatterns
);
const specFiles = await globAsync(specPatterns, excludeSpecPatterns);

const dependsOn: TargetConfiguration['dependsOn'] = [];
const outputs = getOutputs(projectRoot, cypressConfig, 'e2e');
Expand Down
13 changes: 8 additions & 5 deletions packages/eslint/src/plugins/plugin.spec.ts
@@ -1,13 +1,16 @@
import 'nx/src/internal-testing-utils/mock-fs';

let fsRoot: string = '';

jest.mock(
'nx/src/utils/workspace-context',
(): Partial<typeof import('nx/src/utils/workspace-context')> => {
const glob = require('fast-glob');
'nx/src/utils/globs',
(): Partial<typeof import('nx/src/utils/globs')> => {
const glob: typeof import('fast-glob') = require('fast-glob');
return {
globWithWorkspaceContext(workspaceRoot: string, patterns: string[]) {
...jest.requireActual('nx/src/utils/globs'),
globAsync(patterns: string[]) {
// This glob will operate on memfs thanks to 'nx/src/internal-testing-utils/mock-fs'
return glob.sync(patterns, { cwd: workspaceRoot });
return glob(patterns, { cwd: fsRoot });
},
};
}
Expand Down
98 changes: 60 additions & 38 deletions packages/eslint/src/plugins/plugin.ts
Expand Up @@ -7,7 +7,7 @@ import {
import { existsSync } from 'node:fs';
import { dirname, join } from 'node:path';
import { combineGlobPatterns } from 'nx/src/utils/globs';
import { globWithWorkspaceContext } from 'nx/src/utils/workspace-context';
import { globAsync } from 'nx/src/utils/globs';
import {
ESLINT_CONFIG_FILENAMES,
baseEsLintConfigFile,
Expand All @@ -28,60 +28,82 @@ export const createNodes: CreateNodes<EslintPluginOptions> = [
baseEsLintConfigFile,
baseEsLintFlatConfigFile,
]),
(configFilePath, options, context) => {
async (configFilePath, options, context) => {
options = normalizeOptions(options);

// Ensure that configFiles are set, e2e-run fails due to them being undefined in CI (does not occur locally)
// TODO(JamesHenry): Further troubleshoot this in CI
(context as any).configFiles = context.configFiles ?? [];

// Create a Set of all the directories containing eslint configs
const eslintRoots = new Set(context.configFiles.map(dirname));
const configDir = dirname(configFilePath);

const childProjectRoots = globWithWorkspaceContext(
context.workspaceRoot,
[
'project.json',
'package.json',
'**/project.json',
'**/package.json',
].map((f) => join(configDir, f))
)
.map((f) => dirname(f))
.filter((childProjectRoot) => {
// Filter out projects under other eslint configs
let root = childProjectRoot;
// Traverse up from the childProjectRoot to either the workspaceRoot or the dir of this config file
while (root !== dirname(root) && root !== dirname(configFilePath)) {
if (eslintRoots.has(root)) {
return false;
}
root = dirname(root);
}
return true;
})
.filter((dir) => {
// Ignore project roots where the project does not contain any lintable files
const lintableFiles = globWithWorkspaceContext(context.workspaceRoot, [
join(dir, `**/*.{${options.extensions.join(',')}}`),
]);
return lintableFiles.length > 0;
});

const uniqueChildProjectRoots = Array.from(new Set(childProjectRoots));
const childProjectRoots = await getChildProjectRoots(
configFilePath,
context,
options
);

return {
projects: getProjectsUsingESLintConfig(
configFilePath,
uniqueChildProjectRoots,
childProjectRoots,
options,
context
),
};
},
];

async function getChildProjectRoots(
configFilePath: string,
context: CreateNodesContext,
options: EslintPluginOptions
) {
// Create a Set of all the directories containing eslint configs
const eslintRoots = new Set(context.configFiles.map(dirname));
const configDir = dirname(configFilePath);

const projectFiles = await globAsync(
['project.json', 'package.json', '**/project.json', '**/package.json'].map(
(f) => join(configDir, f)
)
);

const childProjectRoots = new Set<string>();
for (const projectFile of projectFiles) {
const dir = dirname(projectFile);

// Filter out projects under other eslint configs
if (hasParentEslintConfig(dir, eslintRoots, configFilePath)) {
continue;
}

// Ignore project roots where the project does not contain any lintable files
const lintableFiles = await globAsync([
join(dir, `**/*.{${options.extensions.join(',')}}`),
]);
if (lintableFiles.length > 0) {
childProjectRoots.add(dir);
}
}

return Array.from(childProjectRoots);
}

function hasParentEslintConfig(
dir: string,
eslintRoots: Set<string>,
configFilePath: string
): boolean {
let root = dir;
// Traverse up from the childProjectRoot to either the workspaceRoot or the dir of this config file
while (root !== dirname(root) && root !== dirname(configFilePath)) {
if (eslintRoots.has(root)) {
return true;
}
root = dirname(root);
}
return false;
}

function getProjectsUsingESLintConfig(
configFilePath: string,
childProjectRoots: string[],
Expand Down
13 changes: 13 additions & 0 deletions packages/nx/src/daemon/client/client.ts
Expand Up @@ -29,10 +29,14 @@ import {
DaemonProjectGraphError,
ProjectGraphError,
} from '../../project-graph/error-types';
import { HandleGlobMessage } from '../message-types/glob';

const DAEMON_ENV_SETTINGS = {
NX_PROJECT_GLOB_CACHE: 'false',
NX_CACHE_PROJECTS_CONFIG: 'false',

// Used to identify that the code is running in the daemon process.
NX_ON_DAEMON_PROCESS: 'true',
Copy link
Collaborator

Choose a reason for hiding this comment

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

I think we can set a global.NX_DAEMON = true on the Daemon Server

};

export type UnregisterCallback = () => void;
Expand Down Expand Up @@ -238,6 +242,15 @@ export class DaemonClient {
});
}

glob(globs: string[], exclude?: string[]): Promise<string[]> {
const message: HandleGlobMessage = {
type: 'GLOB',
globs,
exclude,
};
return this.sendToDaemonViaQueue(message);
}

async isServerAvailable(): Promise<boolean> {
return new Promise((resolve) => {
try {
Expand Down
18 changes: 18 additions & 0 deletions packages/nx/src/daemon/message-types/glob.ts
@@ -0,0 +1,18 @@
export const GLOB = 'GLOB' as const;

export type HandleGlobMessage = {
type: typeof GLOB;
globs: string[];
exclude?: string[];
};

export function isHandleGlobMessage(
message: unknown
): message is HandleGlobMessage {
return (
typeof message === 'object' &&
message !== null &&
'type' in message &&
message['type'] === 'GLOB'
);
}
14 changes: 14 additions & 0 deletions packages/nx/src/daemon/server/handle-glob.ts
@@ -0,0 +1,14 @@
import { workspaceRoot } from '../../utils/workspace-root';
import { globWithWorkspaceContext } from '../../utils/workspace-context';
import { HandlerResult } from './server';

export async function handleGlob(
globs: string[],
exclude?: string[]
): Promise<HandlerResult> {
const files = await globWithWorkspaceContext(workspaceRoot, globs, exclude);
return {
response: JSON.stringify(files),
description: 'handleGlob',
};
}
6 changes: 6 additions & 0 deletions packages/nx/src/daemon/server/server.ts
Expand Up @@ -52,6 +52,8 @@ import {
watchOutputFiles,
watchWorkspace,
} from './watcher';
import { handleGlob } from './handle-glob';
import { GLOB, isHandleGlobMessage } from '../message-types/glob';

let performanceObserver: PerformanceObserver | undefined;
let workspaceWatcherError: Error | undefined;
Expand Down Expand Up @@ -165,6 +167,10 @@ async function handleMessage(socket, data: string) {
);
} else if (payload.type === 'REGISTER_FILE_WATCHER') {
registeredFileWatcherSockets.push({ socket, config: payload.config });
} else if (isHandleGlobMessage(payload)) {
await handleResult(socket, GLOB, () =>
handleGlob(payload.globs, payload.exclude)
);
} else {
await respondWithErrorAndExit(
socket,
Expand Down
23 changes: 14 additions & 9 deletions packages/nx/src/project-graph/plugins/isolation/plugin-pool.ts
Expand Up @@ -28,17 +28,22 @@ export function loadRemoteNxPlugin(
// but its typescript.
const isWorkerTypescript = path.extname(__filename) === '.ts';
const workerPath = path.join(__dirname, 'plugin-worker');

const env: Record<string, string> = {
...process.env,
...(isWorkerTypescript
? {
// Ensures that the worker uses the same tsconfig as the main process
TS_NODE_PROJECT: path.join(__dirname, '../../../tsconfig.lib.json'),
}
: {}),
};

delete env.NX_ON_DAEMON_PROCESS;

const worker = fork(workerPath, [], {
stdio: ['ignore', 'inherit', 'inherit', 'ipc'],
env: {
...process.env,
...(isWorkerTypescript
? {
// Ensures that the worker uses the same tsconfig as the main process
TS_NODE_PROJECT: path.join(__dirname, '../../../tsconfig.lib.json'),
}
: {}),
},
env,
execArgv: [
...process.execArgv,
// If the worker is typescript, we need to register ts-node
Expand Down
13 changes: 13 additions & 0 deletions packages/nx/src/utils/globs.ts
@@ -1,4 +1,17 @@
import { daemonClient } from '../daemon/client/client';
import { isOnDaemon } from './is-on-daemon';
import { globWithWorkspaceContext } from './workspace-context';
import { workspaceRoot } from './workspace-root';

export function combineGlobPatterns(...patterns: (string | string[])[]) {
const p = patterns.flat();
return p.length > 1 ? '{' + p.join(',') + '}' : p.length === 1 ? p[0] : '';
}

export async function globAsync(globs: string[], exclude?: string[]) {
if (isOnDaemon() || !daemonClient.enabled()) {
return globWithWorkspaceContext(workspaceRoot, globs, exclude);
} else {
return daemonClient.glob(globs, exclude);
}
}
Comment on lines +11 to +17
Copy link
Collaborator

Choose a reason for hiding this comment

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

I think we can do this in the workspace-context.ts file.

Those functions should actually all go to the daemon...

Things like allWorkspaceFiles are also broken right now.

Could we change them all?

3 changes: 3 additions & 0 deletions packages/nx/src/utils/is-on-daemon.ts
@@ -0,0 +1,3 @@
export function isOnDaemon() {
return !!process.env.NX_ON_DAEMON_PROCESS;
}
Comment on lines +1 to +3
Copy link
Collaborator

Choose a reason for hiding this comment

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

This should probably live in the daemon directory?