Skip to content

Commit

Permalink
fix(cli): append .cmd for npx & npm (#158)
Browse files Browse the repository at this point in the history
Windows platform requires `npx` and `npm` cannot be directly invoked with `spawn`. To invoke them with `spawn` we now use the `*.cmd` executable. (this is probably due to the command interpreter on windows automagically appending `.cmd` when relevant.
In the same vein, `ui:create:vue` has been fixed: Windows cannot spawn 'shebanged' NodeJS script directly (e.g. `spawn('foo.js')`). It needs to invoke it by passing the script path to node (e.g. `spawn(node, 'foo.js')`)

https://coveord.atlassian.net/browse/CDX-245
  • Loading branch information
louis-bompart authored Apr 17, 2021
1 parent 9a4743c commit e67238b
Show file tree
Hide file tree
Showing 18 changed files with 133 additions and 19 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,7 @@ function getEnvVariables() {

function startServer() {
const serverPath = join(process.cwd(), 'server');
const child = spawnSync('npm', ['run', 'start'], {
const child = spawnSync(appendCmdIfWindows`npm`, ['run', 'start'], {
stdio: 'inherit',
env: getEnvVariables(),
cwd: resolve(serverPath),
Expand All @@ -20,6 +20,9 @@ function startServer() {
}
}

const appendCmdIfWindows = (cmd) =>
`${cmd}${process.platform === 'win32' ? '.cmd' : ''}`;

function main() {
startServer();
}
Expand Down
33 changes: 31 additions & 2 deletions packages/cli/src/commands/ui/create/react.ts
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,7 @@ import {
IsNodeVersionInRange,
IsNpxInstalled,
} from '../../../lib/decorators/preconditions';
import {appendCmdIfWindows} from '../../../lib/utils/os';

export default class React extends Command {
static templateName = '@coveo/cra-template';
Expand Down Expand Up @@ -54,6 +55,7 @@ export default class React extends Command {
const args = this.args;

await this.createProject(args.name);
await this.setupServer(args.name);
await this.setupEnvironmentVariables(args.name);
await this.config.runHook('analytics', buildAnalyticsSuccessHook(this, {}));
}
Expand Down Expand Up @@ -83,7 +85,7 @@ export default class React extends Command {
const {userInfo, apiKey} = await this.platformUserCredentials();

const output = await spawnProcessOutput(
'npm',
appendCmdIfWindows`npm`,
[
'run',
'setup-env',
Expand Down Expand Up @@ -116,8 +118,35 @@ export default class React extends Command {
return true;
}

private async setupServer(name: string) {
const output = await spawnProcessOutput(
appendCmdIfWindows`npm`,
['run', 'setup-server'],
{
cwd: name,
}
);

if (output.exitCode) {
this.warn(`
An unknown error happened while trying to copy the search token server. Please refer to ${join(
name,
'README.md'
)} for more detail.
${output.stderr ?? ''}
`);
return false;
}

return true;
}

private runReactCliCommand(args: string[], options = {}) {
return spawnProcess('npx', ['create-react-app'].concat(args), options);
return spawnProcess(
appendCmdIfWindows`npx`,
['create-react-app'].concat(args),
options
);
}

private get configuration() {
Expand Down
13 changes: 11 additions & 2 deletions packages/cli/src/commands/ui/create/vue.ts
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,7 @@ import {Config} from '../../../lib/config/config';
import {
Preconditions,
IsAuthenticated,
IsNodeVersionInRange,
} from '../../../lib/decorators/preconditions';
import {AuthenticatedClient} from '../../../lib/platform/authenticatedClient';
import {platformUrl} from '../../../lib/platform/environment';
Expand All @@ -17,6 +18,11 @@ import {getPackageVersion} from '../../../lib/utils/misc';
export default class Vue extends Command {
static templateName = '@coveo/vue-cli-plugin-typescript';

/**
* @see https://cli.vuejs.org/guide/installation.html for current requirements.
* @see https://github.com/vuejs/vue-cli/blob/dev/CHANGELOG.md for upcoming requirements.
*/
static requiredNodeVersion = '>=12';
static description =
'Create a Coveo Headless-powered search page with the Vue.js web framework. See https://docs.coveo.com/headless and https://vuejs.org/';

Expand Down Expand Up @@ -47,7 +53,10 @@ export default class Vue extends Command {
{name: 'name', description: 'The target application name.', required: true},
];

@Preconditions(IsAuthenticated())
@Preconditions(
IsAuthenticated(),
IsNodeVersionInRange(Vue.requiredNodeVersion)
)
async run() {
const {args, flags} = this.parse(Vue);

Expand Down Expand Up @@ -145,7 +154,7 @@ export default class Vue extends Command {

private runVueCliCommand(args: string[], options = {}) {
const executable = require.resolve('@vue/cli/bin/vue.js');
return spawnProcess(executable, args, options);
return spawnProcess('node', [executable, ...args], options);
}

private get configuration() {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -62,12 +62,12 @@ export function getBinVersionPrecondition(

export function getBinInstalledPrecondition(
binaryName: string,
options?: Omit<IBinPreconditionsOptions, 'prettyName'>
options?: IBinPreconditionsOptions
) {
const appliedOptions: Required<IBinPreconditionsOptions> = {
...defaultOptions,
...options,
prettyName: binaryName,
prettyName: options?.prettyName ?? binaryName,
};
return function () {
return async function (target: Command) {
Expand All @@ -92,7 +92,7 @@ function isBinInstalled(
return false;
}

if (output.stderr || output.exitCode !== 0) {
if (output.exitCode !== 0) {
target.warn(dedent`
${target.id} requires a valid ${options.prettyName} installation to run.
An unknown error happened while running ${binaryName} ${options.params.join(
Expand Down
3 changes: 2 additions & 1 deletion packages/cli/src/lib/decorators/preconditions/npm.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,7 @@ import {spawnProcessOutput} from '../../utils/process';
import {getFakeCommand} from './testsUtils/utils';

import {IsNpmVersionInRange} from './npm';
import {appendCmdIfWindows} from '../../utils/os';

describe('IsNpmVersionInRange', () => {
const mockedSpawnProcessOutput = mocked(spawnProcessOutput);
Expand Down Expand Up @@ -70,7 +71,7 @@ describe('IsNpmVersionInRange', () => {
expect(fakeCommand.warn).toHaveBeenCalledTimes(2);
expect(fakeCommand.warn).toHaveBeenCalledWith(dedent`
foo requires a valid npm installation to run.
An unknown error happened while running npm --version.
An unknown error happened while running ${appendCmdIfWindows`npm`} --version.
some random error oh no
`);
expect(fakeCommand.warn).toHaveBeenCalledWith(dedent`
Expand Down
6 changes: 5 additions & 1 deletion packages/cli/src/lib/decorators/preconditions/npm.ts
Original file line number Diff line number Diff line change
@@ -1,3 +1,7 @@
import {appendCmdIfWindows} from '../../utils/os';
import {getBinVersionPrecondition} from './binPreconditionsFactory';

export const IsNpmVersionInRange = getBinVersionPrecondition('npm');
export const IsNpmVersionInRange = getBinVersionPrecondition(
appendCmdIfWindows`npm`,
{prettyName: 'npm'}
);
3 changes: 2 additions & 1 deletion packages/cli/src/lib/decorators/preconditions/npx.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,7 @@ import {spawnProcessOutput} from '../../utils/process';
import {getFakeCommand} from './testsUtils/utils';

import {IsNpxInstalled} from './npx';
import {appendCmdIfWindows} from '../../utils/os';

describe('IsNpxInstalled', () => {
const mockedSpawnProcessOutput = mocked(spawnProcessOutput);
Expand Down Expand Up @@ -61,7 +62,7 @@ describe('IsNpxInstalled', () => {
expect(fakeCommand.warn).toHaveBeenCalledTimes(3);
expect(fakeCommand.warn).toHaveBeenCalledWith(dedent`
foo requires a valid npx installation to run.
An unknown error happened while running npx --version.
An unknown error happened while running ${appendCmdIfWindows`npx`} --version.
some random error oh no
`);
expect(fakeCommand.warn).toHaveBeenCalledWith(dedent`
Expand Down
11 changes: 8 additions & 3 deletions packages/cli/src/lib/decorators/preconditions/npx.ts
Original file line number Diff line number Diff line change
@@ -1,5 +1,10 @@
import {appendCmdIfWindows} from '../../utils/os';
import {getBinInstalledPrecondition} from './binPreconditionsFactory';

export const IsNpxInstalled = getBinInstalledPrecondition('npx', {
howToInstallBinText: 'Newer version Node.js comes bundled with npx.',
});
export const IsNpxInstalled = getBinInstalledPrecondition(
appendCmdIfWindows`npx`,
{
prettyName: 'npx',
howToInstallBinText: 'Newer version Node.js comes bundled with npx.',
}
);
30 changes: 30 additions & 0 deletions packages/cli/src/lib/utils/os.spec.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,30 @@
import {appendCmdIfWindows} from './os';

describe('appendCmdIfWindows', () => {
let originalProcess: PropertyDescriptor | undefined;
afterEach(() => {
originalProcess = Object.getOwnPropertyDescriptor(process, 'platform');
jest.resetAllMocks();
});
afterAll(() => {
Object.defineProperty(process, 'platform', originalProcess!);
jest.clearAllMocks();
});

it('should append cmd if process.platform is win32', () => {
Object.defineProperty(process, 'platform', {
writable: false,
value: 'win32',
});

return expect(appendCmdIfWindows`foo`).toEqual('foo.cmd');
});

it('should append cmd if process.platform is not win32', () => {
Object.defineProperty(process, 'platform', {
writable: false,
value: 'linux',
});
return expect(appendCmdIfWindows`foo`).toEqual('foo');
});
});
2 changes: 2 additions & 0 deletions packages/cli/src/lib/utils/os.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,2 @@
export const appendCmdIfWindows = (cmd: TemplateStringsArray) =>
`${cmd}${process.platform === 'win32' ? '.cmd' : ''}`;
3 changes: 2 additions & 1 deletion packages/cra-template/template.json
Original file line number Diff line number Diff line change
Expand Up @@ -21,9 +21,10 @@
},
"proxy": "http://localhost:4000",
"scripts": {
"postinstall": "node ./scripts/setup-server.js",
"setup-server": "node ./scripts/setup-server.js",
"setup-env": "node ./scripts/setup-env.js",
"start-server": "node ./scripts/start-server.js",
"prestart": "node ./scripts/ensure-server.js",
"start": "concurrently --raw \"npm run start-server\" \"react-scripts start\""
}
}
Expand Down
15 changes: 15 additions & 0 deletions packages/cra-template/template/scripts/ensure-server.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,15 @@
const {accessSync, constants} = require('fs');
const {resolve, join} = require('path');
const {setupSearchTokenServer} = require('./setup-server');

function main() {
accessSync(
resolve(join(process.cwd(), 'server', 'package.json')),
constants.F_OK,
() => {
setupSearchTokenServer();
}
);
}

main();
5 changes: 4 additions & 1 deletion packages/cra-template/template/scripts/setup-server.js
Original file line number Diff line number Diff line change
@@ -1,9 +1,10 @@
const {spawnSync} = require('child_process');
const {copySync} = require('fs-extra');
const {sep, resolve} = require('path');
const {appendCmdIfWindows} = require('./utils');

function installSearchTokenServerDependencies() {
const child = spawnSync('npm', ['install'], {
const child = spawnSync(appendCmdIfWindows`npm`, ['install'], {
stdio: 'inherit',
cwd: resolve('server'),
});
Expand Down Expand Up @@ -36,3 +37,5 @@ function main() {
}

main();

module.exports = {setupSearchTokenServer: main};
3 changes: 2 additions & 1 deletion packages/cra-template/template/scripts/start-server.js
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
const {spawn} = require('child_process');
const {resolve, join} = require('path');
const {config} = require('dotenv');
const {appendCmdIfWindows} = require('./utils');
config();

function getEnvVariables() {
Expand All @@ -20,7 +21,7 @@ function getEnvVariables() {

function startServer() {
const serverPath = join(process.cwd(), 'server');
const child = spawn('npm', ['run', 'start'], {
const child = spawn(appendCmdIfWindows`npm`, ['run', 'start'], {
stdio: 'inherit',
env: getEnvVariables(),
cwd: resolve(serverPath),
Expand Down
4 changes: 4 additions & 0 deletions packages/cra-template/template/scripts/utils.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,4 @@
const appendCmdIfWindows = (cmd) =>
`${cmd}${process.platform === 'win32' ? '.cmd' : ''}`;

module.exports = {appendCmdIfWindows};
Original file line number Diff line number Diff line change
@@ -1,9 +1,10 @@
const {spawnSync} = require('child_process');
const {copySync} = require('fs-extra');
const {sep, resolve} = require('path');
const {appendCmdIfWindows} = require('./utils');

function installSearchTokenServerDependencies() {
const child = spawnSync('npm', ['install'], {
const child = spawnSync(appendCmdIfWindows`npm`, ['install'], {
stdio: 'inherit',
cwd: resolve('server'),
});
Expand Down
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
const {spawn} = require('child_process');
const {resolve, join} = require('path');
const {config} = require('dotenv');
const {appendCmdIfWindows} = require('./utils');
config();

function getEnvVariables() {
Expand All @@ -22,7 +23,7 @@ function getEnvVariables() {

function startServer() {
const serverPath = join(process.cwd(), 'server');
const child = spawn('npm', ['run', 'start'], {
const child = spawn(appendCmdIfWindows`npm`, ['run', 'start'], {
stdio: 'inherit',
env: getEnvVariables(),
cwd: resolve(serverPath),
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,4 @@
const appendCmdIfWindows = (cmd) =>
`${cmd}${process.platform === 'win32' ? '.cmd' : ''}`;

module.exports = {appendCmdIfWindows};

0 comments on commit e67238b

Please sign in to comment.