Skip to content

Commit

Permalink
fix(testing): migrations should look for existing target before attem…
Browse files Browse the repository at this point in the history
…pting migration (#27441)

<!-- 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 -->
Migrations are naively setting `serve-static` or `preview` as the target
without checking if that target exists


## Expected Behavior
<!-- This is the behavior we should expect with the changes in this PR
-->
Check if the target exists based on executor usage as a last port of
call, and use the name of the target it is found on

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

Closes #27439
  • Loading branch information
Coly010 authored Aug 14, 2024
1 parent 4d27939 commit 3d7f60a
Show file tree
Hide file tree
Showing 4 changed files with 244 additions and 29 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -211,6 +211,54 @@ export default defineConfig({
"
`);
});

it('should use @nx/vite:preview-server executor target value if it exists when no plugins are found', async () => {
// ARRANGE
const nxJson = readNxJson(tree);
nxJson.plugins = [
{
plugin: '@nx/cypress/plugin',
options: {
targetName: 'e2e',
ciTargetName: 'e2e-ci',
},
},
];
updateNxJson(tree, nxJson);

addProject(tree, tempFs, {
appName: 'app',
ciTargetName: 'e2e-ci',
buildTargetName: 'build',
usesExecutors: true,
});

// ACT
await updateCiWebserverForVite(tree);

// ASSERT
expect(tree.read('app-e2e/cypress.config.ts', 'utf-8'))
.toMatchInlineSnapshot(`
"import { nxE2EPreset } from '@nx/cypress/plugins/cypress-preset';
import { defineConfig } from 'cypress';
export default defineConfig({
e2e: {
...nxE2EPreset(__filename, {
cypressDir: 'src',
bundler: 'vite',
webServerCommands: {
default: 'nx run app:serve',
production: 'nx run app:preview',
},
ciWebServerCommand: 'nx run app:test-serve-static',
ciBaseUrl: 'http://localhost:4300',
}),
baseUrl: 'http://localhost:4200',
},
});
"
`);
});
});

function addProject(
Expand All @@ -222,13 +270,25 @@ function addProject(
appName: string;
noCi?: boolean;
noVite?: boolean;
usesExecutors?: boolean;
} = { ciTargetName: 'e2e-ci', buildTargetName: 'build', appName: 'app' }
) {
const appProjectConfig = {
name: overrides.appName,
root: overrides.appName,
sourceRoot: `${overrides.appName}/src`,
projectType: 'application',
...(overrides.usesExecutors
? {
targets: {
'test-serve-static': {
executor: overrides.noVite
? '@nx/web:file-server'
: '@nx/vite:preview-server',
},
},
}
: {}),
};
const viteConfig = `/// <reference types='vitest' />
import { defineConfig } from 'vite';
Expand Down Expand Up @@ -327,12 +387,29 @@ export default defineConfig({
projectType: 'application',
root: overrides.appName,
targets: {
[overrides.buildTargetName]: {},
'serve-static': {
options: {
buildTarget: overrides.buildTargetName,
},
},
...(overrides.usesExecutors
? {
'test-serve-static': {
executor: overrides.noVite
? '@nx/web:file-server'
: '@nx/vite:preview-server',
dependsOn: [overrides.buildTargetName],
},
[overrides.buildTargetName]: {},
'serve-static': {
options: {
buildTarget: overrides.buildTargetName,
},
},
}
: {
[overrides.buildTargetName]: {},
'serve-static': {
options: {
buildTarget: overrides.buildTargetName,
},
},
}),
},
},
};
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,7 @@ import {
PluginConfiguration,
CreateNodes,
formatFiles,
type ProjectGraph,
} from '@nx/devkit';
import {
retrieveProjectConfigurations,
Expand Down Expand Up @@ -106,7 +107,16 @@ export default async function (tree: Tree) {
? 'serve-static'
: (matchingWebpackPlugin.options as any)?.serveStaticTargetName ??
'serve-static'
: 'serve-static';
: getServeStaticLikeTarget(
tree,
graph,
project,
'@nx/web:file-server'
) ?? undefined;

if (!serveStaticTargetName) {
continue;
}

const newCommand = ciWebServerCommand.replace(
/nx.*[^"']/,
Expand Down Expand Up @@ -135,7 +145,16 @@ export default async function (tree: Tree) {
? 'preview'
: (matchingVitePlugin.options as any)?.previewTargetName ??
'preview'
: 'preview';
: getServeStaticLikeTarget(
tree,
graph,
project,
'@nx/vite:preview-server'
) ?? undefined;

if (!previewTargetName) {
continue;
}

const newCommand = ciWebServerCommand.replace(
/nx.*[^"']/,
Expand Down Expand Up @@ -199,3 +218,22 @@ async function findPluginForConfigFile(
}
}
}

function getServeStaticLikeTarget(
tree: Tree,
graph: ProjectGraph,
projectName: string,
executorName: string
) {
if (!graph.nodes[projectName]?.data?.targets) {
return;
}

for (const [targetName, targetOptions] of Object.entries(
graph.nodes[projectName].data.targets
)) {
if (targetOptions.executor && targetOptions.executor === executorName) {
return targetName;
}
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -32,7 +32,7 @@ describe('useServeStaticPreviewForCommand', () => {
jest.resetModules();
});

it('should update when it does not use serve-static for non-vite', async () => {
it('should not assume a default if it cannot find a plugin or executor', async () => {
// ARRANGE
addProject(tree, tempFs, { noVite: true });

Expand All @@ -56,7 +56,7 @@ describe('useServeStaticPreviewForCommand', () => {
trace: 'on-first-retry',
},
webServer: {
command: 'npx nx run app:serve-static',
command: 'npx nx run app:serve',
url: 'http://localhost:4200',
reuseExistingServer: !process.env.CI,
cwd: workspaceRoot,
Expand Down Expand Up @@ -224,6 +224,49 @@ describe('useServeStaticPreviewForCommand', () => {
`);
});

it('should update command to be target name for preview executor', async () => {
// ARRANGE
addProject(tree, tempFs, {
hasAdditionalCommand: true,
usesExecutors: true,
});

// ACT
await useServeStaticPreviewForCommand(tree);

// ASSERT
expect(tree.read('app-e2e/playwright.config.ts', 'utf-8'))
.toMatchInlineSnapshot(`
"import { defineConfig, devices } from '@playwright/test';
import { nxE2EPreset } from '@nx/playwright/preset';
import { workspaceRoot } from '@nx/devkit';
const baseURL = process.env['BASE_URL'] || 'http://localhost:4300';
export default defineConfig({
...nxE2EPreset(__filename, { testDir: './src' }),
use: {
baseURL,
trace: 'on-first-retry',
},
webServer: {
command: 'echo "start" && npx nx run app:test-serve-static',
url: 'http://localhost:4300',
reuseExistingServer: !process.env.CI,
cwd: workspaceRoot,
},
projects: [
{
name: 'chromium',
use: { ...devices['Desktop Chrome'] },
},
],
});
"
`);
});

function mockWebpackConfig(config: any) {
jest.mock(join(tempFs.tempDir, 'app/webpack.config.ts'), () => config, {
virtual: true,
Expand Down Expand Up @@ -299,13 +342,25 @@ function addProject(
overrides: {
noVite?: boolean;
hasAdditionalCommand?: boolean;
usesExecutors?: boolean;
} = {}
) {
const appProjectConfig = {
name: 'app',
root: 'app',
sourceRoot: `${'app'}/src`,
projectType: 'application',
...(overrides.usesExecutors
? {
targets: {
'test-serve-static': {
executor: overrides.noVite
? '@nx/web:file-server'
: '@nx/vite:preview-server',
},
},
}
: {}),
};

const e2eProjectConfig = {
Expand Down Expand Up @@ -357,7 +412,18 @@ function addProject(
data: {
projectType: 'application',
root: 'app',
targets: {},
targets: {
...(overrides.usesExecutors
? {
'test-serve-static': {
dependsOn: ['build'],
executor: overrides.noVite
? '@nx/web:file-server'
: '@nx/vite:preview-server',
},
}
: {}),
},
},
};

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@ import {
getPackageManagerCommand,
joinPathFragments,
parseTargetString,
ProjectGraph,
readNxJson,
type Tree,
visitNotIgnoredFiles,
Expand Down Expand Up @@ -110,22 +111,36 @@ export default async function (tree: Tree) {
projectToMigrate.playwrightConfigFile,
'utf-8'
);
const targetName = await getServeStaticTargetNameForConfigFile(
tree,
projectToMigrate.configFileType === 'webpack'
? '@nx/webpack/plugin'
: '@nx/vite/plugin',
projectToMigrate.configFile,
projectToMigrate.configFileType === 'webpack'
? 'serve-static'
: 'preview',
projectToMigrate.configFileType === 'webpack'
? 'serveStaticTargetName'
: 'previewTargetName',
projectToMigrate.configFileType === 'webpack'
? webpackCreateNodesV2
: viteCreateNodesV2
);
const targetName =
(await getServeStaticTargetNameForConfigFile(
tree,
projectToMigrate.configFileType === 'webpack'
? '@nx/webpack/plugin'
: '@nx/vite/plugin',
projectToMigrate.configFile,
projectToMigrate.configFileType === 'webpack'
? 'serve-static'
: 'preview',
projectToMigrate.configFileType === 'webpack'
? 'serveStaticTargetName'
: 'previewTargetName',
projectToMigrate.configFileType === 'webpack'
? webpackCreateNodesV2
: viteCreateNodesV2
)) ??
getServeStaticLikeTarget(
tree,
graph,
projectToMigrate.projectName,
projectToMigrate.configFileType === 'webpack'
? '@nx/web:file-server'
: '@nx/vite:preview-server'
);

if (!targetName) {
continue;
}

const oldCommand = projectToMigrate.commandValueNode.getText();
const newCommand = oldCommand.replace(
/nx.*[^"']/,
Expand Down Expand Up @@ -225,10 +240,10 @@ async function getServeStaticTargetNameForConfigFile<T>(
);

if (!matchingPluginRegistrations) {
return defaultTargetName;
return undefined;
}

let targetName = defaultTargetName;
let targetName = undefined;
for (const plugin of matchingPluginRegistrations) {
let projectConfigs: ConfigurationResult;
try {
Expand Down Expand Up @@ -259,3 +274,22 @@ async function getServeStaticTargetNameForConfigFile<T>(
}
return targetName;
}

function getServeStaticLikeTarget(
tree: Tree,
graph: ProjectGraph,
projectName: string,
executorName: string
) {
if (!graph.nodes[projectName]?.data?.targets) {
return;
}

for (const [targetName, targetOptions] of Object.entries(
graph.nodes[projectName].data.targets
)) {
if (targetOptions.executor && targetOptions.executor === executorName) {
return targetName;
}
}
}

0 comments on commit 3d7f60a

Please sign in to comment.