Skip to content
This repository was archived by the owner on Dec 16, 2022. It is now read-only.

feat(cli): make app path optional #457

Merged
merged 4 commits into from
Jun 28, 2021
Merged

feat(cli): make app path optional #457

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
b2ddd73
feat(cli): make app path optional
eunjae-lee Jun 23, 2021
cb20f32
fix: do not ask project name if given by argument
eunjae-lee Jun 23, 2021
5763d5e
fix: update build script for CI test
eunjae-lee Jun 23, 2021
7c42b52
Apply suggestions from code review
Jun 28, 2021
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
7 changes: 6 additions & 1 deletion e2e/installs.test.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@ const { execSync } = require('child_process');
describe('Installation', () => {
let temporaryDirectory;
let appPath;
const appName = 'test-app';

beforeAll(() => {
temporaryDirectory = execSync(
Expand All @@ -18,7 +19,7 @@ describe('Installation', () => {
});

beforeEach(() => {
appPath = `${temporaryDirectory}/test-app`;
appPath = `${temporaryDirectory}/${appName}`;
execSync(`mkdir ${appPath}`);
});

Expand All @@ -30,6 +31,7 @@ describe('Installation', () => {
test('get skipped with the `no-installation` flag', () => {
execSync(
`yarn start ${appPath} \
--name ${appName} \
Copy link
Contributor Author

@eunjae-lee eunjae-lee Jun 23, 2021

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

To avoid the prompt, we need to pass the app name explicitly in the tests.

--template "InstantSearch.js" \
--no-installation`,
{ stdio: 'ignore' }
Expand All @@ -43,6 +45,7 @@ describe('Installation', () => {
test('without conflict generates files', () => {
execSync(
`yarn start ${appPath} \
--name ${appName} \
--template "InstantSearch.js" \
--no-installation`,
{ stdio: 'ignore' }
Expand All @@ -57,6 +60,7 @@ describe('Installation', () => {
expect(() => {
execSync(
`yarn start ${appPath} \
--name ${appName} \
--template "InstantSearch.js" \
--no-installation`,
{ stdio: 'ignore' }
Expand All @@ -76,6 +80,7 @@ describe('Installation', () => {
expect(() => {
execSync(
`yarn start ${appPath}/file \
--name ${appName} \
--template "InstantSearch.js" \
--no-installation`,
{ stdio: 'ignore' }
Expand Down
1 change: 1 addition & 0 deletions e2e/templates.test.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -57,6 +57,7 @@ describe('Templates', () => {

execSync(
`yarn start ${appPath} \
--name ${templateConfig.appName} \
--config ${configFilePath} \
--no-installation`,
{ stdio: 'ignore' }
Expand Down
3 changes: 3 additions & 0 deletions scripts/build-app.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -18,8 +18,11 @@ if (!templateName) {
process.exit(1);
}

const appName = path.basename(appPath);

execSync(
`yarn start ${appPath} \
--name "${appName}" \
--template "${templateName}"`,
{ stdio: 'inherit' }
);
Expand Down
95 changes: 54 additions & 41 deletions src/cli/index.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@ const program = require('commander');
const inquirer = require('inquirer');
const chalk = require('chalk');
const latestSemver = require('latest-semver');
const os = require('os');

const createInstantSearchApp = require('../api');
const {
Expand All @@ -25,14 +26,14 @@ const {
} = require('./getConfiguration');
const { version } = require('../../package.json');

let appPath;
let appPathFromArgument;
let options = {};

program
.version(version, '-v, --version')
.arguments('<project-directory>')
.usage(`${chalk.green('<project-directory>')} [options]`)
.option('--name <name>', 'The name of the application')
.option('--name <name>', 'The name of the application or widget')
.option('--app-id <appId>', 'The application ID')
.option('--api-key <apiKey>', 'The Algolia search API key')
.option('--index-name <indexName>', 'The main index of your search')
Expand All @@ -49,52 +50,18 @@ program
.option('--config <config>', 'The configuration file to get the options from')
.option('--no-installation', 'Ignore dependency installation')
.action((dest, opts) => {
appPath = dest;
appPathFromArgument = dest;
options = opts;
})
.parse(process.argv);

if (!appPath) {
console.log('Please specify the project directory:');
console.log();
console.log(
` ${chalk.cyan('create-instantsearch-app')} ${chalk.green(
'<project-directory>'
)}`
);
console.log();
console.log('For example:');
console.log(
` ${chalk.cyan('create-instantsearch-app')} ${chalk.green(
'my-instantsearch-app'
)}`
);
console.log();
console.log(
`Run ${chalk.cyan('create-instantsearch-app --help')} to see all options.`
);

process.exit(1);
}

const optionsFromArguments = getOptionsFromArguments(options.rawArgs);
const appName = optionsFromArguments.name || path.basename(appPath);
const optionsFromArguments = getOptionsFromArguments(options.rawArgs || []);
const attributesToDisplay = (optionsFromArguments.attributesToDisplay || '')
.split(',')
.filter(Boolean)
.map(x => x.trim());

try {
checkAppPath(appPath);
checkAppName(appName);
} catch (err) {
console.error(err.message);
console.log();

process.exit(1);
}

const questions = {
const getQuestions = ({ appName }) => ({
application: [
{
type: 'list',
Expand Down Expand Up @@ -212,9 +179,55 @@ const questions = {
},
},
],
};
});

async function run() {
let appPath = appPathFromArgument;
if (!appPath) {
const answers = await inquirer.prompt([
{
type: 'input',
name: 'appPath',
message: 'Project directory',
},
]);
appPath = answers.appPath;
}
if (appPath.startsWith('~/')) {
Copy link
Contributor Author

@eunjae-lee eunjae-lee Jun 23, 2021

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Unlike command-line argument, the string path including ~/ needs to be expanded by ourselves.

Copy link
Contributor

@francoischalifour francoischalifour Jun 28, 2021

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

How does it work if it starts with / or ./? Perhaps we should use a Regex instead.

Copy link
Contributor

@Haroenv Haroenv Jun 28, 2021

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

is there a way to forward the path expanding to the shell instead?

Copy link
Contributor

@Haroenv Haroenv Jun 28, 2021

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If it's a problem, there's also https://nodejs.org/api/path.html#path_path_relative_from_to

Copy link
Contributor Author

@eunjae-lee eunjae-lee Jun 28, 2021

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@francoischalifour @Haroenv
In the end, it gets resolved here.

> path.resolve('/test')
'/test'
> path.resolve('./test')
'/Users/eunjaelee/workspace/create-instantsearch-app/test'
> path.resolve('~/test')
'/Users/eunjaelee/workspace/create-instantsearch-app/~/test'
>

If a path starts with / or ./, path.resolve() works correctly. However ~ didn't work, because, I guess, it has no context of home directory, or something.

Copy link
Contributor

@Haroenv Haroenv Jun 28, 2021

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for checking Eunjae!

appPath = path.join(os.homedir(), appPath.slice(2));
}
try {
checkAppPath(appPath);
} catch (err) {
console.error(err.message);
console.log();

process.exit(1);
}

let appName = optionsFromArguments.name;
if (!appName) {
appName = (
await inquirer.prompt([
{
type: 'input',
name: 'appName',
message: 'The name of the application or widget',
default: path.basename(appPath),
},
])
).appName;
}

try {
checkAppName(appName);
} catch (err) {
console.error(err.message);
console.log();

process.exit(1);
}

console.log();
console.log(`Creating a new InstantSearch app in ${chalk.green(appPath)}.`);
console.log();
Expand Down Expand Up @@ -264,7 +277,7 @@ async function run() {
templateConfig.category === 'Widget' ? 'widget' : 'application';

const answers = await inquirer.prompt(
questions[implementationType].filter(question =>
getQuestions({ appName })[implementationType].filter(question =>
isQuestionAsked({ question, args: optionsFromArguments })
),
{ ...optionsFromArguments, template }
Expand Down