Skip to content

Commit

Permalink
fix(web): spa flag should correctly define redirect (#22487)
Browse files Browse the repository at this point in the history
  • Loading branch information
Coly010 committed Mar 26, 2024
1 parent 02075d5 commit 29c80a3
Show file tree
Hide file tree
Showing 27 changed files with 95 additions and 37 deletions.
2 changes: 1 addition & 1 deletion docs/generated/packages/web/executors/file-server.json
Original file line number Diff line number Diff line change
Expand Up @@ -47,7 +47,7 @@
},
"proxyUrl": {
"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": {
"type": "object",
Expand Down
5 changes: 5 additions & 0 deletions docs/generated/packages/web/generators/static-config.json
Original file line number Diff line number Diff line change
Expand Up @@ -21,6 +21,11 @@
"type": "string",
"description": "Name of the serve target to add. Defaults to 'serve-static'.",
"default": "serve-static"
},
"spa": {
"type": "boolean",
"description": "Whether to set the 'spa' flag on the generated target.",
"default": true
}
},
"required": ["buildTarget"],
Expand Down
2 changes: 1 addition & 1 deletion e2e/storybook-angular/src/storybook-angular.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -38,7 +38,7 @@ describe('Storybook executors for Angular', () => {
}
);
p.kill();
}, 200_000);
}, 300_000);

// 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.
Expand Down
2 changes: 1 addition & 1 deletion e2e/web/src/file-server-legacy.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -36,7 +36,7 @@ describe('file-server', () => {
}
);
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: {
NX_ADD_PLUGINS: 'false',
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -287,6 +287,7 @@ exports[`app --project-name-and-root-format=derived should generate correctly wh
"executor": "@nx/web:file-server",
"options": {
"buildTarget": "my-dir-my-app:build",
"spa": true,
"staticFilePath": "dist/apps/my-dir/my-app/browser",
},
},
Expand Down Expand Up @@ -486,6 +487,7 @@ exports[`app --project-name-and-root-format=derived should generate correctly wh
"executor": "@nx/web:file-server",
"options": {
"buildTarget": "my-app:build",
"spa": true,
"staticFilePath": "dist/apps/my-app/browser",
},
},
Expand Down Expand Up @@ -980,6 +982,7 @@ exports[`app nested should create project configs 1`] = `
"executor": "@nx/web:file-server",
"options": {
"buildTarget": "my-app:build",
"spa": true,
"staticFilePath": "dist/my-dir/my-app/browser",
},
},
Expand Down Expand Up @@ -1092,6 +1095,7 @@ exports[`app not nested should create project configs 1`] = `
"executor": "@nx/web:file-server",
"options": {
"buildTarget": "my-app:build",
"spa": true,
"staticFilePath": "dist/my-app/browser",
},
},
Expand Down
1 change: 1 addition & 0 deletions packages/angular/src/generators/application/lib/add-e2e.ts
Original file line number Diff line number Diff line change
Expand Up @@ -94,6 +94,7 @@ function addFileServerTarget(
staticFilePath: isUsingApplicationBuilder
? joinPathFragments(options.outputPath, 'browser')
: undefined,
spa: true,
},
};
updateProjectConfiguration(tree, options.name, projectConfig);
Expand Down
2 changes: 1 addition & 1 deletion packages/expo/src/generators/application/lib/add-e2e.ts
Original file line number Diff line number Diff line change
Expand Up @@ -21,7 +21,7 @@ export async function addE2e(
case 'cypress': {
const hasNxExpoPlugin = hasExpoPlugin(tree);
if (!hasNxExpoPlugin) {
webStaticServeGenerator(tree, {
await webStaticServeGenerator(tree, {
buildTarget: `${options.projectName}:export`,
targetName: 'serve-static',
});
Expand Down
3 changes: 2 additions & 1 deletion packages/next/src/generators/application/lib/add-e2e.ts
Original file line number Diff line number Diff line change
Expand Up @@ -26,10 +26,11 @@ export async function addE2e(host: Tree, options: NormalizedSchema) {
>('@nx/cypress', nxVersion);

if (!hasPlugin) {
webStaticServeGenerator(host, {
await webStaticServeGenerator(host, {
buildTarget: `${options.projectName}:build`,
outputPath: `${options.outputPath}/out`,
targetName: 'serve-static',
spa: true,
});
}

Expand Down
2 changes: 2 additions & 0 deletions packages/next/src/plugins/__snapshots__/plugin.spec.ts.snap
Original file line number Diff line number Diff line change
Expand Up @@ -40,6 +40,7 @@ exports[`@nx/next/plugin integrated projects should create nodes 1`] = `
"options": {
"buildTarget": "my-build",
"port": 3000,
"spa": false,
"staticFilePath": "{projectRoot}/out",
},
},
Expand Down Expand Up @@ -98,6 +99,7 @@ exports[`@nx/next/plugin root projects should create nodes 1`] = `
"options": {
"buildTarget": "build",
"port": 3000,
"spa": false,
"staticFilePath": "{projectRoot}/out",
},
},
Expand Down
2 changes: 2 additions & 0 deletions packages/next/src/plugins/plugin.ts
Original file line number Diff line number Diff line change
Expand Up @@ -162,6 +162,8 @@ function getStaticServeTargetConfig(options: NextPluginOptions) {
buildTarget: options.buildTargetName,
staticFilePath: '{projectRoot}/out',
port: 3000,
// Routes are found correctly with serve-static
spa: false,
},
};

Expand Down
2 changes: 2 additions & 0 deletions packages/nuxt/src/plugins/__snapshots__/plugin.spec.ts.snap
Original file line number Diff line number Diff line change
Expand Up @@ -33,6 +33,7 @@ exports[`@nx/nuxt/plugin not root project should create nodes 1`] = `
"options": {
"buildTarget": "acme-build-static",
"port": 4200,
"spa": false,
"staticFilePath": "{projectRoot}/dist",
},
},
Expand Down Expand Up @@ -131,6 +132,7 @@ exports[`@nx/nuxt/plugin root project should create nodes 1`] = `
"options": {
"buildTarget": "build-static",
"port": 4200,
"spa": false,
"staticFilePath": "{projectRoot}/dist",
},
},
Expand Down
2 changes: 2 additions & 0 deletions packages/nuxt/src/plugins/plugin.ts
Original file line number Diff line number Diff line change
Expand Up @@ -166,6 +166,8 @@ function serveStaticTarget(options: NuxtPluginOptions) {
buildTarget: `${options.buildStaticTargetName}`,
staticFilePath: '{projectRoot}/dist',
port: 4200,
// Routes are found correctly with serve-static
spa: false,
},
};

Expand Down
3 changes: 2 additions & 1 deletion packages/react/src/generators/application/lib/add-e2e.ts
Original file line number Diff line number Diff line change
Expand Up @@ -22,9 +22,10 @@ export async function addE2e(
(options.bundler === 'webpack' && hasWebpackPlugin(tree)) ||
(options.bundler === 'vite' && hasVitePlugin(tree));
if (!hasNxBuildPlugin) {
webStaticServeGenerator(tree, {
await webStaticServeGenerator(tree, {
buildTarget: `${options.projectName}:build`,
targetName: 'serve-static',
spa: true,
});
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -154,7 +154,6 @@ export default defineConfig({
...nxE2EPreset(__filename, {
cypressDir: 'src',
webServerCommands: { default: 'nx run test:serve:development' },
ciWebServerCommand: 'nx run test:serve-static',
}),
baseUrl: 'http://localhost:4200',
},
Expand Down Expand Up @@ -668,7 +667,6 @@ export default defineConfig({
...nxE2EPreset(__filename, {
cypressDir: 'src',
webServerCommands: { default: 'nx run test:serve:development' },
ciWebServerCommand: 'nx run test:serve-static',
}),
baseUrl: 'http://localhost:4200',
},
Expand Down Expand Up @@ -1038,7 +1036,6 @@ export default defineConfig({
...nxE2EPreset(__filename, {
cypressDir: 'src',
webServerCommands: { default: 'nx run test:serve:development' },
ciWebServerCommand: 'nx run test:serve-static',
}),
baseUrl: 'http://localhost:4200',
},
Expand Down
4 changes: 3 additions & 1 deletion packages/remix/src/generators/application/lib/add-e2e.ts
Original file line number Diff line number Diff line change
Expand Up @@ -16,7 +16,9 @@ export async function addE2E(tree: Tree, options: NormalizedSchema) {
typeof import('@nx/cypress')
>('@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, {
projectType: 'application',
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -186,7 +186,7 @@ export async function configurationGeneratorInternal(
);
}
if (schema.configureStaticServe) {
addStaticTarget(tree, schema);
await addStaticTarget(tree, schema);
}
} else {
devDeps['storybook'] = storybookVersion;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -135,9 +135,15 @@ export function addAngularStorybookTarget(
updateProjectConfiguration(tree, projectName, projectConfig);
}

export function addStaticTarget(tree: Tree, opts: StorybookConfigureSchema) {
const nrwlWeb = ensurePackage<typeof import('@nx/web')>('@nx/web', nxVersion);
nrwlWeb.webStaticServeGenerator(tree, {
export async function addStaticTarget(
tree: Tree,
opts: StorybookConfigureSchema
) {
const { webStaticServeGenerator } = ensurePackage<typeof import('@nx/web')>(
'@nx/web',
nxVersion
);
await webStaticServeGenerator(tree, {
buildTarget: `${opts.project}:build-storybook`,
outputPath: joinPathFragments('dist/storybook', opts.project),
targetName: 'static-storybook',
Expand Down
2 changes: 2 additions & 0 deletions packages/vite/src/plugins/__snapshots__/plugin.spec.ts.snap
Original file line number Diff line number Diff line change
Expand Up @@ -44,6 +44,7 @@ exports[`@nx/vite/plugin not root project should create nodes 1`] = `
"executor": "@nx/web:file-server",
"options": {
"buildTarget": "build-something",
"spa": true,
},
},
},
Expand Down Expand Up @@ -96,6 +97,7 @@ exports[`@nx/vite/plugin root project should create nodes 1`] = `
"executor": "@nx/web:file-server",
"options": {
"buildTarget": "build",
"spa": true,
},
},
},
Expand Down
1 change: 1 addition & 0 deletions packages/vite/src/plugins/plugin.ts
Original file line number Diff line number Diff line change
Expand Up @@ -226,6 +226,7 @@ function serveStaticTarget(options: VitePluginOptions) {
executor: '@nx/web:file-server',
options: {
buildTarget: `${options.buildTargetName}`,
spa: true,
},
};

Expand Down
3 changes: 2 additions & 1 deletion packages/vue/src/generators/application/lib/add-e2e.ts
Original file line number Diff line number Diff line change
Expand Up @@ -24,9 +24,10 @@ export async function addE2e(
: p.plugin === '@nx/vite/plugin'
);
if (!hasPlugin) {
webStaticServeGenerator(tree, {
await webStaticServeGenerator(tree, {
buildTarget: `${options.projectName}:build`,
targetName: 'serve-static',
spa: true,
});
}

Expand Down
6 changes: 5 additions & 1 deletion packages/web/src/executors/file-server/file-server.impl.ts
Original file line number Diff line number Diff line change
Expand Up @@ -181,6 +181,7 @@ export default async function* fileServerExecutor(
run();
}

const port = await detectPort(options.port || 8080);
const outputPath = getBuildTargetOutputPath(options, context);

if (options.spa) {
Expand All @@ -189,6 +190,10 @@ export default async function* fileServerExecutor(

// See: https://github.com/http-party/http-server#magic-files
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);
Expand All @@ -205,7 +210,6 @@ export default async function* fileServerExecutor(

// detect port as close to when used to prevent port being used by another process
// when running in parallel
const port = await detectPort(options.port || 8080);
args.push(`-p=${port}`);

const serve = fork(pathToHttpServer, [outputPath, ...args], {
Expand Down
2 changes: 1 addition & 1 deletion packages/web/src/executors/file-server/schema.json
Original file line number Diff line number Diff line change
Expand Up @@ -44,7 +44,7 @@
},
"proxyUrl": {
"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": {
"type": "object",
Expand Down
5 changes: 5 additions & 0 deletions packages/web/src/generators/static-serve/schema.json
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,11 @@
"type": "string",
"description": "Name of the serve target to add. Defaults to 'serve-static'.",
"default": "serve-static"
},
"spa": {
"type": "boolean",
"description": "Whether to set the 'spa' flag on the generated target.",
"default": true
}
},
"required": ["buildTarget"]
Expand Down
Loading

0 comments on commit 29c80a3

Please sign in to comment.