fix(web): spa flag should correctly define redirect (#22487)

This commit is contained in:
Colum Ferry 2024-03-26 09:51:03 -07:00 committed by GitHub
parent 02075d5aed
commit 29c80a33de
No known key found for this signature in database
GPG Key ID: B5690EEEBB952194
27 changed files with 95 additions and 37 deletions

View File

@ -47,7 +47,7 @@
}, },
"proxyUrl": { "proxyUrl": {
"type": "string", "type": "string",
"description": "URL to proxy unhandled requests to." "description": "URL to proxy unhandled requests to. _Note: If the 'spa' flag is set to true, manually setting this value will override the catch-all redirect functionality from http-server which may lead to unexpected behavior._"
}, },
"proxyOptions": { "proxyOptions": {
"type": "object", "type": "object",

View File

@ -21,6 +21,11 @@
"type": "string", "type": "string",
"description": "Name of the serve target to add. Defaults to 'serve-static'.", "description": "Name of the serve target to add. Defaults to 'serve-static'.",
"default": "serve-static" "default": "serve-static"
},
"spa": {
"type": "boolean",
"description": "Whether to set the 'spa' flag on the generated target.",
"default": true
} }
}, },
"required": ["buildTarget"], "required": ["buildTarget"],

View File

@ -38,7 +38,7 @@ describe('Storybook executors for Angular', () => {
} }
); );
p.kill(); p.kill();
}, 200_000); }, 300_000);
// Increased timeout because 92% sealing asset processing TerserPlugin // Increased timeout because 92% sealing asset processing TerserPlugin
// TODO(meeroslav) this test is still flaky and breaks the PR runs. We need to investigate why. // TODO(meeroslav) this test is still flaky and breaks the PR runs. We need to investigate why.

View File

@ -36,7 +36,7 @@ describe('file-server', () => {
} }
); );
runCLI( runCLI(
`generate @nx/web:static-config --buildTarget=${ngAppName}:build --no-interactive`, `generate @nx/web:static-config --buildTarget=${ngAppName}:build --outputPath=dist/apps/${ngAppName}/browser --no-interactive`,
{ {
env: { env: {
NX_ADD_PLUGINS: 'false', NX_ADD_PLUGINS: 'false',

View File

@ -287,6 +287,7 @@ exports[`app --project-name-and-root-format=derived should generate correctly wh
"executor": "@nx/web:file-server", "executor": "@nx/web:file-server",
"options": { "options": {
"buildTarget": "my-dir-my-app:build", "buildTarget": "my-dir-my-app:build",
"spa": true,
"staticFilePath": "dist/apps/my-dir/my-app/browser", "staticFilePath": "dist/apps/my-dir/my-app/browser",
}, },
}, },
@ -486,6 +487,7 @@ exports[`app --project-name-and-root-format=derived should generate correctly wh
"executor": "@nx/web:file-server", "executor": "@nx/web:file-server",
"options": { "options": {
"buildTarget": "my-app:build", "buildTarget": "my-app:build",
"spa": true,
"staticFilePath": "dist/apps/my-app/browser", "staticFilePath": "dist/apps/my-app/browser",
}, },
}, },
@ -980,6 +982,7 @@ exports[`app nested should create project configs 1`] = `
"executor": "@nx/web:file-server", "executor": "@nx/web:file-server",
"options": { "options": {
"buildTarget": "my-app:build", "buildTarget": "my-app:build",
"spa": true,
"staticFilePath": "dist/my-dir/my-app/browser", "staticFilePath": "dist/my-dir/my-app/browser",
}, },
}, },
@ -1092,6 +1095,7 @@ exports[`app not nested should create project configs 1`] = `
"executor": "@nx/web:file-server", "executor": "@nx/web:file-server",
"options": { "options": {
"buildTarget": "my-app:build", "buildTarget": "my-app:build",
"spa": true,
"staticFilePath": "dist/my-app/browser", "staticFilePath": "dist/my-app/browser",
}, },
}, },

View File

@ -94,6 +94,7 @@ function addFileServerTarget(
staticFilePath: isUsingApplicationBuilder staticFilePath: isUsingApplicationBuilder
? joinPathFragments(options.outputPath, 'browser') ? joinPathFragments(options.outputPath, 'browser')
: undefined, : undefined,
spa: true,
}, },
}; };
updateProjectConfiguration(tree, options.name, projectConfig); updateProjectConfiguration(tree, options.name, projectConfig);

View File

@ -21,7 +21,7 @@ export async function addE2e(
case 'cypress': { case 'cypress': {
const hasNxExpoPlugin = hasExpoPlugin(tree); const hasNxExpoPlugin = hasExpoPlugin(tree);
if (!hasNxExpoPlugin) { if (!hasNxExpoPlugin) {
webStaticServeGenerator(tree, { await webStaticServeGenerator(tree, {
buildTarget: `${options.projectName}:export`, buildTarget: `${options.projectName}:export`,
targetName: 'serve-static', targetName: 'serve-static',
}); });

View File

@ -26,10 +26,11 @@ export async function addE2e(host: Tree, options: NormalizedSchema) {
>('@nx/cypress', nxVersion); >('@nx/cypress', nxVersion);
if (!hasPlugin) { if (!hasPlugin) {
webStaticServeGenerator(host, { await webStaticServeGenerator(host, {
buildTarget: `${options.projectName}:build`, buildTarget: `${options.projectName}:build`,
outputPath: `${options.outputPath}/out`, outputPath: `${options.outputPath}/out`,
targetName: 'serve-static', targetName: 'serve-static',
spa: true,
}); });
} }

View File

@ -40,6 +40,7 @@ exports[`@nx/next/plugin integrated projects should create nodes 1`] = `
"options": { "options": {
"buildTarget": "my-build", "buildTarget": "my-build",
"port": 3000, "port": 3000,
"spa": false,
"staticFilePath": "{projectRoot}/out", "staticFilePath": "{projectRoot}/out",
}, },
}, },
@ -98,6 +99,7 @@ exports[`@nx/next/plugin root projects should create nodes 1`] = `
"options": { "options": {
"buildTarget": "build", "buildTarget": "build",
"port": 3000, "port": 3000,
"spa": false,
"staticFilePath": "{projectRoot}/out", "staticFilePath": "{projectRoot}/out",
}, },
}, },

View File

@ -162,6 +162,8 @@ function getStaticServeTargetConfig(options: NextPluginOptions) {
buildTarget: options.buildTargetName, buildTarget: options.buildTargetName,
staticFilePath: '{projectRoot}/out', staticFilePath: '{projectRoot}/out',
port: 3000, port: 3000,
// Routes are found correctly with serve-static
spa: false,
}, },
}; };

View File

@ -33,6 +33,7 @@ exports[`@nx/nuxt/plugin not root project should create nodes 1`] = `
"options": { "options": {
"buildTarget": "acme-build-static", "buildTarget": "acme-build-static",
"port": 4200, "port": 4200,
"spa": false,
"staticFilePath": "{projectRoot}/dist", "staticFilePath": "{projectRoot}/dist",
}, },
}, },
@ -131,6 +132,7 @@ exports[`@nx/nuxt/plugin root project should create nodes 1`] = `
"options": { "options": {
"buildTarget": "build-static", "buildTarget": "build-static",
"port": 4200, "port": 4200,
"spa": false,
"staticFilePath": "{projectRoot}/dist", "staticFilePath": "{projectRoot}/dist",
}, },
}, },

View File

@ -166,6 +166,8 @@ function serveStaticTarget(options: NuxtPluginOptions) {
buildTarget: `${options.buildStaticTargetName}`, buildTarget: `${options.buildStaticTargetName}`,
staticFilePath: '{projectRoot}/dist', staticFilePath: '{projectRoot}/dist',
port: 4200, port: 4200,
// Routes are found correctly with serve-static
spa: false,
}, },
}; };

View File

@ -22,9 +22,10 @@ export async function addE2e(
(options.bundler === 'webpack' && hasWebpackPlugin(tree)) || (options.bundler === 'webpack' && hasWebpackPlugin(tree)) ||
(options.bundler === 'vite' && hasVitePlugin(tree)); (options.bundler === 'vite' && hasVitePlugin(tree));
if (!hasNxBuildPlugin) { if (!hasNxBuildPlugin) {
webStaticServeGenerator(tree, { await webStaticServeGenerator(tree, {
buildTarget: `${options.projectName}:build`, buildTarget: `${options.projectName}:build`,
targetName: 'serve-static', targetName: 'serve-static',
spa: true,
}); });
} }

View File

@ -154,7 +154,6 @@ export default defineConfig({
...nxE2EPreset(__filename, { ...nxE2EPreset(__filename, {
cypressDir: 'src', cypressDir: 'src',
webServerCommands: { default: 'nx run test:serve:development' }, webServerCommands: { default: 'nx run test:serve:development' },
ciWebServerCommand: 'nx run test:serve-static',
}), }),
baseUrl: 'http://localhost:4200', baseUrl: 'http://localhost:4200',
}, },
@ -668,7 +667,6 @@ export default defineConfig({
...nxE2EPreset(__filename, { ...nxE2EPreset(__filename, {
cypressDir: 'src', cypressDir: 'src',
webServerCommands: { default: 'nx run test:serve:development' }, webServerCommands: { default: 'nx run test:serve:development' },
ciWebServerCommand: 'nx run test:serve-static',
}), }),
baseUrl: 'http://localhost:4200', baseUrl: 'http://localhost:4200',
}, },
@ -1038,7 +1036,6 @@ export default defineConfig({
...nxE2EPreset(__filename, { ...nxE2EPreset(__filename, {
cypressDir: 'src', cypressDir: 'src',
webServerCommands: { default: 'nx run test:serve:development' }, webServerCommands: { default: 'nx run test:serve:development' },
ciWebServerCommand: 'nx run test:serve-static',
}), }),
baseUrl: 'http://localhost:4200', baseUrl: 'http://localhost:4200',
}, },

View File

@ -16,7 +16,9 @@ export async function addE2E(tree: Tree, options: NormalizedSchema) {
typeof import('@nx/cypress') typeof import('@nx/cypress')
>('@nx/cypress', getPackageVersion(tree, 'nx')); >('@nx/cypress', getPackageVersion(tree, 'nx'));
addFileServerTarget(tree, options, 'serve-static'); // TODO(colum): Remix needs a different approach to serve-static
// Likely via remix start
// addFileServerTarget(tree, options, 'serve-static');
addProjectConfiguration(tree, options.e2eProjectName, { addProjectConfiguration(tree, options.e2eProjectName, {
projectType: 'application', projectType: 'application',

View File

@ -186,7 +186,7 @@ export async function configurationGeneratorInternal(
); );
} }
if (schema.configureStaticServe) { if (schema.configureStaticServe) {
addStaticTarget(tree, schema); await addStaticTarget(tree, schema);
} }
} else { } else {
devDeps['storybook'] = storybookVersion; devDeps['storybook'] = storybookVersion;

View File

@ -135,9 +135,15 @@ export function addAngularStorybookTarget(
updateProjectConfiguration(tree, projectName, projectConfig); updateProjectConfiguration(tree, projectName, projectConfig);
} }
export function addStaticTarget(tree: Tree, opts: StorybookConfigureSchema) { export async function addStaticTarget(
const nrwlWeb = ensurePackage<typeof import('@nx/web')>('@nx/web', nxVersion); tree: Tree,
nrwlWeb.webStaticServeGenerator(tree, { opts: StorybookConfigureSchema
) {
const { webStaticServeGenerator } = ensurePackage<typeof import('@nx/web')>(
'@nx/web',
nxVersion
);
await webStaticServeGenerator(tree, {
buildTarget: `${opts.project}:build-storybook`, buildTarget: `${opts.project}:build-storybook`,
outputPath: joinPathFragments('dist/storybook', opts.project), outputPath: joinPathFragments('dist/storybook', opts.project),
targetName: 'static-storybook', targetName: 'static-storybook',

View File

@ -44,6 +44,7 @@ exports[`@nx/vite/plugin not root project should create nodes 1`] = `
"executor": "@nx/web:file-server", "executor": "@nx/web:file-server",
"options": { "options": {
"buildTarget": "build-something", "buildTarget": "build-something",
"spa": true,
}, },
}, },
}, },
@ -96,6 +97,7 @@ exports[`@nx/vite/plugin root project should create nodes 1`] = `
"executor": "@nx/web:file-server", "executor": "@nx/web:file-server",
"options": { "options": {
"buildTarget": "build", "buildTarget": "build",
"spa": true,
}, },
}, },
}, },

View File

@ -226,6 +226,7 @@ function serveStaticTarget(options: VitePluginOptions) {
executor: '@nx/web:file-server', executor: '@nx/web:file-server',
options: { options: {
buildTarget: `${options.buildTargetName}`, buildTarget: `${options.buildTargetName}`,
spa: true,
}, },
}; };

View File

@ -24,9 +24,10 @@ export async function addE2e(
: p.plugin === '@nx/vite/plugin' : p.plugin === '@nx/vite/plugin'
); );
if (!hasPlugin) { if (!hasPlugin) {
webStaticServeGenerator(tree, { await webStaticServeGenerator(tree, {
buildTarget: `${options.projectName}:build`, buildTarget: `${options.projectName}:build`,
targetName: 'serve-static', targetName: 'serve-static',
spa: true,
}); });
} }

View File

@ -181,6 +181,7 @@ export default async function* fileServerExecutor(
run(); run();
} }
const port = await detectPort(options.port || 8080);
const outputPath = getBuildTargetOutputPath(options, context); const outputPath = getBuildTargetOutputPath(options, context);
if (options.spa) { if (options.spa) {
@ -189,6 +190,10 @@ export default async function* fileServerExecutor(
// See: https://github.com/http-party/http-server#magic-files // See: https://github.com/http-party/http-server#magic-files
copyFileSync(src, dst); copyFileSync(src, dst);
// We also need to ensure the proxyUrl is set, otherwise the browser will continue to throw a 404 error
// This can cause unexpected behaviors and failures especially in automated test suites
options.proxyUrl ??= `http${options.ssl ? 's' : ''}://localhost:${port}?`;
} }
const args = getHttpServerArgs(options); const args = getHttpServerArgs(options);
@ -205,7 +210,6 @@ export default async function* fileServerExecutor(
// detect port as close to when used to prevent port being used by another process // detect port as close to when used to prevent port being used by another process
// when running in parallel // when running in parallel
const port = await detectPort(options.port || 8080);
args.push(`-p=${port}`); args.push(`-p=${port}`);
const serve = fork(pathToHttpServer, [outputPath, ...args], { const serve = fork(pathToHttpServer, [outputPath, ...args], {

View File

@ -44,7 +44,7 @@
}, },
"proxyUrl": { "proxyUrl": {
"type": "string", "type": "string",
"description": "URL to proxy unhandled requests to." "description": "URL to proxy unhandled requests to. _Note: If the 'spa' flag is set to true, manually setting this value will override the catch-all redirect functionality from http-server which may lead to unexpected behavior._"
}, },
"proxyOptions": { "proxyOptions": {
"type": "object", "type": "object",

View File

@ -18,6 +18,11 @@
"type": "string", "type": "string",
"description": "Name of the serve target to add. Defaults to 'serve-static'.", "description": "Name of the serve target to add. Defaults to 'serve-static'.",
"default": "serve-static" "default": "serve-static"
},
"spa": {
"type": "boolean",
"description": "Whether to set the 'spa' flag on the generated target.",
"default": true
} }
}, },
"required": ["buildTarget"] "required": ["buildTarget"]

View File

@ -13,12 +13,12 @@ describe('Static serve configuration generator', () => {
tree = createTreeWithEmptyWorkspace(); tree = createTreeWithEmptyWorkspace();
}); });
it('should add a `serve-static` target to the project', () => { it('should add a `serve-static` target to the project', async () => {
addReactConfig(tree, 'react-app'); addReactConfig(tree, 'react-app');
addAngularConfig(tree, 'angular-app'); addAngularConfig(tree, 'angular-app');
addStorybookConfig(tree, 'storybook'); addStorybookConfig(tree, 'storybook');
webStaticServeGenerator(tree, { await webStaticServeGenerator(tree, {
buildTarget: 'react-app:build', buildTarget: 'react-app:build',
}); });
@ -28,10 +28,11 @@ describe('Static serve configuration generator', () => {
"executor": "@nx/web:file-server", "executor": "@nx/web:file-server",
"options": { "options": {
"buildTarget": "react-app:build", "buildTarget": "react-app:build",
"spa": true,
}, },
} }
`); `);
webStaticServeGenerator(tree, { await webStaticServeGenerator(tree, {
buildTarget: 'angular-app:build', buildTarget: 'angular-app:build',
}); });
@ -42,11 +43,12 @@ describe('Static serve configuration generator', () => {
"executor": "@nx/web:file-server", "executor": "@nx/web:file-server",
"options": { "options": {
"buildTarget": "angular-app:build", "buildTarget": "angular-app:build",
"spa": true,
}, },
} }
`); `);
webStaticServeGenerator(tree, { await webStaticServeGenerator(tree, {
buildTarget: 'storybook:build-storybook', buildTarget: 'storybook:build-storybook',
}); });
expect(readProjectConfiguration(tree, 'storybook').targets['serve-static']) expect(readProjectConfiguration(tree, 'storybook').targets['serve-static'])
@ -55,15 +57,16 @@ describe('Static serve configuration generator', () => {
"executor": "@nx/web:file-server", "executor": "@nx/web:file-server",
"options": { "options": {
"buildTarget": "storybook:build-storybook", "buildTarget": "storybook:build-storybook",
"spa": true,
"staticFilePath": "dist/storybook/storybook", "staticFilePath": "dist/storybook/storybook",
}, },
} }
`); `);
}); });
it('should support custom target name', () => { it('should support custom target name', async () => {
addReactConfig(tree, 'react-app'); addReactConfig(tree, 'react-app');
webStaticServeGenerator(tree, { await webStaticServeGenerator(tree, {
buildTarget: 'react-app:build', buildTarget: 'react-app:build',
targetName: 'serve-static-custom', targetName: 'serve-static-custom',
}); });
@ -75,12 +78,13 @@ describe('Static serve configuration generator', () => {
"executor": "@nx/web:file-server", "executor": "@nx/web:file-server",
"options": { "options": {
"buildTarget": "react-app:build", "buildTarget": "react-app:build",
"spa": true,
}, },
} }
`); `);
}); });
it('should infer outputPath via the buildTarget#outputs', () => { it('should infer outputPath via the buildTarget#outputs', async () => {
addAngularConfig(tree, 'angular-app'); addAngularConfig(tree, 'angular-app');
const projectConfig = readProjectConfiguration(tree, 'angular-app'); const projectConfig = readProjectConfiguration(tree, 'angular-app');
delete projectConfig.targets.build.options.outputPath; delete projectConfig.targets.build.options.outputPath;
@ -89,7 +93,7 @@ describe('Static serve configuration generator', () => {
updateProjectConfiguration(tree, 'angular-app', projectConfig); updateProjectConfiguration(tree, 'angular-app', projectConfig);
webStaticServeGenerator(tree, { await webStaticServeGenerator(tree, {
buildTarget: 'angular-app:build', buildTarget: 'angular-app:build',
}); });
@ -100,13 +104,14 @@ describe('Static serve configuration generator', () => {
"executor": "@nx/web:file-server", "executor": "@nx/web:file-server",
"options": { "options": {
"buildTarget": "angular-app:build", "buildTarget": "angular-app:build",
"spa": true,
"staticFilePath": "dist/angular-app", "staticFilePath": "dist/angular-app",
}, },
} }
`); `);
}); });
it('should not override targets', () => { it('should not override targets', async () => {
addStorybookConfig(tree, 'storybook'); addStorybookConfig(tree, 'storybook');
const pc = readProjectConfiguration(tree, 'storybook'); const pc = readProjectConfiguration(tree, 'storybook');
@ -117,10 +122,10 @@ describe('Static serve configuration generator', () => {
updateProjectConfiguration(tree, 'storybook', pc); updateProjectConfiguration(tree, 'storybook', pc);
expect(() => { expect(() => {
webStaticServeGenerator(tree, { return webStaticServeGenerator(tree, {
buildTarget: 'storybook:build-storybook', buildTarget: 'storybook:build-storybook',
}); });
}).toThrowErrorMatchingInlineSnapshot(` }).rejects.toThrowErrorMatchingInlineSnapshot(`
"Project storybook already has a 'serve-static' target configured. "Project storybook already has a 'serve-static' target configured.
Either rename or remove the existing 'serve-static' target and try again. Either rename or remove the existing 'serve-static' target and try again.
Optionally, you can provide a different name with the --target-name option other than 'serve-static'" Optionally, you can provide a different name with the --target-name option other than 'serve-static'"

View File

@ -1,42 +1,54 @@
import { import {
createProjectGraphAsync,
logger, logger,
parseTargetString, parseTargetString,
type ProjectGraph,
readCachedProjectGraph,
readProjectConfiguration, readProjectConfiguration,
stripIndents, stripIndents,
TargetConfiguration, type TargetConfiguration,
Tree, type Tree,
updateProjectConfiguration, updateProjectConfiguration,
} from '@nx/devkit'; } from '@nx/devkit';
import { Schema as FileServerExecutorSchema } from '../../executors/file-server/schema.d'; import type { Schema as FileServerExecutorSchema } from '../../executors/file-server/schema.d';
interface WebStaticServeSchema { interface WebStaticServeSchema {
buildTarget: string; buildTarget: string;
outputPath?: string; outputPath?: string;
targetName?: string; targetName?: string;
spa?: boolean;
} }
interface NormalizedWebStaticServeSchema extends WebStaticServeSchema { interface NormalizedWebStaticServeSchema extends WebStaticServeSchema {
projectName: string; projectName: string;
targetName: string; targetName: string;
spa: boolean;
} }
export function webStaticServeGenerator( export async function webStaticServeGenerator(
tree: Tree, tree: Tree,
options: WebStaticServeSchema options: WebStaticServeSchema
) { ) {
const opts = normalizeOptions(tree, options); const opts = await normalizeOptions(tree, options);
addStaticConfig(tree, opts); addStaticConfig(tree, opts);
} }
function normalizeOptions( async function normalizeOptions(
tree: Tree, tree: Tree,
options: WebStaticServeSchema options: WebStaticServeSchema
): NormalizedWebStaticServeSchema { ): Promise<NormalizedWebStaticServeSchema> {
const target = parseTargetString(options.buildTarget); let projectGraph: ProjectGraph;
try {
projectGraph = readCachedProjectGraph();
} catch (e) {
projectGraph = await createProjectGraphAsync();
}
const target = parseTargetString(options.buildTarget, projectGraph);
const opts: NormalizedWebStaticServeSchema = { const opts: NormalizedWebStaticServeSchema = {
...options, ...options,
targetName: options.targetName || 'serve-static', targetName: options.targetName || 'serve-static',
projectName: target.project, projectName: target.project,
spa: options.spa ?? true,
}; };
const projectConfig = readProjectConfiguration(tree, target.project); const projectConfig = readProjectConfiguration(tree, target.project);
@ -54,7 +66,7 @@ Optionally, you can provide a different name with the --target-name option other
// NOTE: @nx/web:file-server only looks for the outputPath option // NOTE: @nx/web:file-server only looks for the outputPath option
if (!buildTargetConfig.options?.outputPath && !opts.outputPath) { if (!buildTargetConfig.options?.outputPath && !opts.outputPath) {
// attempt to find the suiteable path from the outputs // attempt to find the suitable path from the outputs
let maybeOutputValue: any; let maybeOutputValue: any;
for (const o of buildTargetConfig?.outputs || []) { for (const o of buildTargetConfig?.outputs || []) {
const isInterpolatedOutput = o.trim().startsWith('{options.'); const isInterpolatedOutput = o.trim().startsWith('{options.');
@ -100,6 +112,7 @@ function addStaticConfig(tree: Tree, opts: NormalizedWebStaticServeSchema) {
options: { options: {
buildTarget: opts.buildTarget, buildTarget: opts.buildTarget,
staticFilePath: opts.outputPath, staticFilePath: opts.outputPath,
spa: opts.spa,
}, },
}; };

View File

@ -53,6 +53,7 @@ exports[`@nx/webpack/plugin should create nodes 1`] = `
"executor": "@nx/web:file-server", "executor": "@nx/web:file-server",
"options": { "options": {
"buildTarget": "build-something", "buildTarget": "build-something",
"spa": true,
}, },
}, },
}, },

View File

@ -167,6 +167,7 @@ async function createWebpackTargets(
executor: '@nx/web:file-server', executor: '@nx/web:file-server',
options: { options: {
buildTarget: options.buildTargetName, buildTarget: options.buildTargetName,
spa: true,
}, },
}; };