From a1efd0d76db620dd41dd1d56012dcd74f89df847 Mon Sep 17 00:00:00 2001 From: Patrik Oldsberg Date: Mon, 18 May 2020 11:29:54 +0200 Subject: [PATCH 1/4] packages/cli: nicer handling of waiting for bundler to exit --- packages/cli/src/commands/app/build.ts | 3 --- packages/cli/src/commands/app/serve.ts | 5 ++--- packages/cli/src/commands/plugin/serve.ts | 5 ++--- packages/cli/src/lib/bundler/server.ts | 24 +++++++++++++++-------- 4 files changed, 20 insertions(+), 17 deletions(-) diff --git a/packages/cli/src/commands/app/build.ts b/packages/cli/src/commands/app/build.ts index 482159e773..c654baa439 100644 --- a/packages/cli/src/commands/app/build.ts +++ b/packages/cli/src/commands/app/build.ts @@ -22,7 +22,4 @@ export default async (cmd: Command) => { entry: 'src/index', statsJsonEnabled: cmd.stats, }); - - // Wait for interrupt signal - await new Promise(() => {}); }; diff --git a/packages/cli/src/commands/app/serve.ts b/packages/cli/src/commands/app/serve.ts index 143e4139e5..182e338287 100644 --- a/packages/cli/src/commands/app/serve.ts +++ b/packages/cli/src/commands/app/serve.ts @@ -18,11 +18,10 @@ import { serveBundle } from '../../lib/bundler'; import { Command } from 'commander'; export default async (cmd: Command) => { - await serveBundle({ + const waitForExit = await serveBundle({ entry: 'src/index', checksEnabled: cmd.check, }); - // Wait for interrupt signal - await new Promise(() => {}); + await waitForExit(); }; diff --git a/packages/cli/src/commands/plugin/serve.ts b/packages/cli/src/commands/plugin/serve.ts index 79a540d879..5700992644 100644 --- a/packages/cli/src/commands/plugin/serve.ts +++ b/packages/cli/src/commands/plugin/serve.ts @@ -18,11 +18,10 @@ import { serveBundle } from '../../lib/bundler'; import { Command } from 'commander'; export default async (cmd: Command) => { - await serveBundle({ + const waitForExit = await serveBundle({ entry: 'dev/index', checksEnabled: cmd.check, }); - // Wait for interrupt signal - await new Promise(() => {}); + await waitForExit(); }; diff --git a/packages/cli/src/lib/bundler/server.ts b/packages/cli/src/lib/bundler/server.ts index ea29b04459..3aaf5968f5 100644 --- a/packages/cli/src/lib/bundler/server.ts +++ b/packages/cli/src/lib/bundler/server.ts @@ -29,7 +29,7 @@ export async function serveBundle(options: ServeOptions) { const port = await choosePort(host, defaultPort); if (!port) { - return; + throw new Error(`Invalid or no port set: '${port}'`); } const protocol = yn(process.env.HTTPS, { default: false }) ? 'https' : 'http'; @@ -57,15 +57,23 @@ export async function serveBundle(options: ServeOptions) { return; } - for (const signal of ['SIGINT', 'SIGTERM'] as const) { - process.on(signal, () => { - server.close(); - process.exit(); - }); - } - openBrowser(urls.localUrlForBrowser); resolve(); }); }); + + const waitForExit = async () => { + for (const signal of ['SIGINT', 'SIGTERM'] as const) { + process.on(signal, () => { + server.close(); + // exit instead of resolve. The process is shutting down and resolving a promise here logs an error + process.exit(); + }); + } + + // Block indefinitely and wait for the interrupt signal + return new Promise(() => {}); + }; + + return waitForExit; } From 0242c8a3442f3a74f48569dbcdb7a864516afef1 Mon Sep 17 00:00:00 2001 From: Patrik Oldsberg Date: Mon, 18 May 2020 11:35:08 +0200 Subject: [PATCH 2/4] packages/cli: add more explanations to installWithLocalDeps + remove console.log --- packages/cli/src/lib/tasks.ts | 11 +++++++++-- 1 file changed, 9 insertions(+), 2 deletions(-) diff --git a/packages/cli/src/lib/tasks.ts b/packages/cli/src/lib/tasks.ts index 2fa6220f18..721e1a355b 100644 --- a/packages/cli/src/lib/tasks.ts +++ b/packages/cli/src/lib/tasks.ts @@ -118,8 +118,11 @@ const PATCH_PACKAGES = [ 'theme', ]; +// This runs a `yarn install` task, but with special treatment for e2e tests export async function installWithLocalDeps(dir: string) { - // e2e testing needs special treatment + // This makes us install any package inside this repo as a local file dependency. + // For example, instead of trying to fetch @backstage/core from npm, we point it + // to /packages/core. This makes yarn use a simple file copy to install it instead. if (process.env.BACKSTAGE_E2E_CLI_TEST) { Task.section('Linking packages locally for e2e tests'); @@ -163,6 +166,11 @@ export async function installWithLocalDeps(dir: string) { }); }); + // This takes care of pointing all the installed packages from this repo to + // dist instead of the local src. + // For example node_modules/@backstage/core/packages.json is rewritten to point + // types to dist/index.d.ts and the main:src field is removed. + // Without this we get type checking errors in the e2e test if (process.env.BACKSTAGE_E2E_CLI_TEST) { Task.section('Patchling local dependencies for e2e tests'); @@ -177,7 +185,6 @@ export async function installWithLocalDeps(dir: string) { name, 'package.json', ); - console.log('DEBUG: depJsonPath =', depJsonPath); const depJson = await fs.readJson(depJsonPath); // We want dist to be used for e2e tests From 26e259d7bd793cc7bda229dcd32ebb38c8e2b286 Mon Sep 17 00:00:00 2001 From: Patrik Oldsberg Date: Mon, 18 May 2020 11:41:38 +0200 Subject: [PATCH 3/4] packages/cli: remove old comment in packager --- packages/cli/src/lib/packager/packager.ts | 1 - 1 file changed, 1 deletion(-) diff --git a/packages/cli/src/lib/packager/packager.ts b/packages/cli/src/lib/packager/packager.ts index e6f71e270b..d3e88d8c3d 100644 --- a/packages/cli/src/lib/packager/packager.ts +++ b/packages/cli/src/lib/packager/packager.ts @@ -24,7 +24,6 @@ function formatErrorMessage(error: any) { let msg = ''; if (error.code === 'PLUGIN_ERROR') { - // typescript2 plugin has a complete message with all codeframes if (error.plugin === 'esbuild') { msg += `${error.message}\n\n`; for (const { text, location } of error.errors) { From 9b3e7b345d665a4caa3e5977f2d758823551fe35 Mon Sep 17 00:00:00 2001 From: Patrik Oldsberg Date: Mon, 18 May 2020 11:44:52 +0200 Subject: [PATCH 4/4] packages/cli: clarify jest module mapper config --- packages/cli/config/jest.js | 1 + 1 file changed, 1 insertion(+) diff --git a/packages/cli/config/jest.js b/packages/cli/config/jest.js index e0b82da2bb..3f37a0df4c 100644 --- a/packages/cli/config/jest.js +++ b/packages/cli/config/jest.js @@ -37,6 +37,7 @@ async function getConfig() { // To avoid having to build all deps inside the monorepo before running tests, // we point directory to src/ where applicable. + // For example, @backstage/core = /packages/core/src/index.ts is added to moduleNameMapper for (const pkg of packages) { const mainSrc = pkg.get('main:src'); if (mainSrc) {