From 975be46879b9ed5960bdf8a2c9184ff9e03f8fdb Mon Sep 17 00:00:00 2001 From: Nicholas Hyatt Date: Wed, 27 Jan 2021 13:06:46 -0600 Subject: [PATCH 1/3] fix(Appflow): Fix issue with deploy add and deploy configure commands where sometimes neccessary variables weren't correctly set. --- .../@ionic/cli/src/commands/deploy/add.ts | 27 +++++--- .../cli/src/commands/deploy/configure.ts | 1 + .../@ionic/cli/src/commands/deploy/core.ts | 63 ++++++++++++++++++- 3 files changed, 78 insertions(+), 13 deletions(-) diff --git a/packages/@ionic/cli/src/commands/deploy/add.ts b/packages/@ionic/cli/src/commands/deploy/add.ts index 729e9e437f..52c1e25879 100644 --- a/packages/@ionic/cli/src/commands/deploy/add.ts +++ b/packages/@ionic/cli/src/commands/deploy/add.ts @@ -2,7 +2,6 @@ import { MetadataGroup } from '@ionic/cli-framework'; import { CommandInstanceInfo, CommandLineInputs, CommandLineOptions, CommandMetadata } from '../../definitions'; import { input } from '../../lib/color'; -import { FatalException } from '../../lib/errors'; import { runCommand } from '../../lib/executor'; import { DeployConfCommand } from './core'; @@ -95,20 +94,12 @@ For Cordova projects it just takes care of running the proper Cordova CLI comman } async preRun(inputs: CommandLineInputs, options: CommandLineOptions): Promise { - // check if it is already installed - const alreadyAdded = await this.checkDeployInstalled(); - if (alreadyAdded) { - throw new FatalException( - `Appflow Deploy plugin is already installed.` - ); - } // check if there are native integration installed await this.requireNativeIntegration(); await this.preRunCheckInputs(options); } - async run(inputs: CommandLineInputs, options: CommandLineOptions, runinfo: CommandInstanceInfo): Promise { - const integration = await this.getAppIntegration(); + async addPlugin(options: CommandLineOptions, runinfo: CommandInstanceInfo, integration?: string) { if (integration === 'cordova') { let deployCommand = ['cordova', 'plugin', 'add', 'cordova-plugin-ionic']; const userOptions = this.buildCordovaDeployOptions(options); @@ -125,6 +116,22 @@ For Cordova projects it just takes care of running the proper Cordova CLI comman ); // install the plugin with npm await this.env.shell.run(installer, installerArgs, { stdio: 'inherit' }); + } + + } + + async run(inputs: CommandLineInputs, options: CommandLineOptions, runinfo: CommandInstanceInfo): Promise { + const integration = await this.getAppIntegration(); + + // check if it is already installed + const alreadyAdded = await this.checkDeployInstalled(); + if (!alreadyAdded) { + this.addPlugin(options, runinfo, integration); + } else { + this.env.log.warn("Live Updates plugin already added. Reconfiguring only."); + } + + if (integration === 'capacitor') { // generate the manifest await runCommand(runinfo, ['deploy', 'manifest']); // run capacitor sync diff --git a/packages/@ionic/cli/src/commands/deploy/configure.ts b/packages/@ionic/cli/src/commands/deploy/configure.ts index c72a6add5c..d6b0f3ce68 100644 --- a/packages/@ionic/cli/src/commands/deploy/configure.ts +++ b/packages/@ionic/cli/src/commands/deploy/configure.ts @@ -4,6 +4,7 @@ import { CommandInstanceInfo, CommandMetadata } from '../../definitions'; import { input } from '../../lib/color'; import { FatalException } from '../../lib/errors'; + import { DeployConfCommand } from './core'; export class ConfigureCommand extends DeployConfCommand { diff --git a/packages/@ionic/cli/src/commands/deploy/core.ts b/packages/@ionic/cli/src/commands/deploy/core.ts index 17e6a7914e..6da7dc3f5e 100644 --- a/packages/@ionic/cli/src/commands/deploy/core.ts +++ b/packages/@ionic/cli/src/commands/deploy/core.ts @@ -2,6 +2,7 @@ import { CommandLineOptions, combine, contains, validators } from '@ionic/cli-fr import { pathExists, pathWritable, readFile, writeFile } from '@ionic/utils-fs'; import * as et from 'elementtree'; import * as path from 'path'; +import * as chalk from 'chalk'; import { input, strong } from '../../lib/color'; import { Command } from '../../lib/command'; @@ -36,7 +37,7 @@ export abstract class DeployCoreCommand extends Command { export abstract class DeployConfCommand extends DeployCoreCommand { - protected readonly optionsToPlistKeys = { + protected readonly optionsToPlistKeys: {[key: string]: string} = { 'app-id': 'IonAppId', 'channel-name': 'IonChannelName', 'update-method': 'IonUpdateMethod', @@ -53,6 +54,24 @@ export abstract class DeployConfCommand extends DeployCoreCommand { 'update-api': 'ionic_update_api', }; + protected readonly requiredOptionsDefaults: {[key: string]: string} = { + 'max-store': '2', + 'min-background-duration': '30', + 'update-api': 'https://api.ionicjs.com', + } + + protected readonly requiredOptionsFromPlistVal: {[key: string]: string} = { + 'IonMaxVersions': 'max-store', + 'IonMinBackgroundDuration': 'min-background-duration', + 'IonApi': 'update-api', + } + + protected readonly requiredOptionsFromXmlVal = { + 'ionic_max_versions': 'max-store', + 'ionic_min_background_duration': 'min-background-duration', + 'ionic_update_api': 'update-api', + } + protected async getAppId(): Promise { if (this.project) { return this.project.config.get('id'); @@ -132,7 +151,7 @@ export abstract class DeployConfCommand extends DeployCoreCommand { return false; } if (!plistPath) { - this.env.log.info(`No Capacitor iOS project found.`); + this.env.log.warn(`No ${chalk.bold('Capacitor iOS')} project found you will need to rerun ${chalk.yellow('ionic deploy configure')} if you add it later.`); return false; } // try to load the plist file first @@ -169,6 +188,7 @@ export abstract class DeployConfCommand extends DeployCoreCommand { } // check which options are set (configure might not have all of them set) const setOptions: { [key: string]: string } = {}; + for (const [optionKey, plistKey] of Object.entries(this.optionsToPlistKeys)) { if (options[optionKey]) { setOptions[optionKey] = plistKey; @@ -183,19 +203,39 @@ export abstract class DeployConfCommand extends DeployCoreCommand { const pdictChildren = pdict.getchildren(); // there is no way to refer to a first right sibling in elementtree, so we use flags let removeNextStringTag = false; + let existingRequiredKeys = []; for (const element of pdictChildren) { + + // find required options and keep track of what is already existing + if ((element.tag === 'key') && (element.text) && this.requiredOptionsFromPlistVal[element.text as string] != undefined) { + existingRequiredKeys.push(this.requiredOptionsFromPlistVal[element.text as string]) + } + // we remove all the existing element if there if ((element.tag === 'key') && (element.text) && Object.values(setOptions).includes(element.text as string)) { pdict.remove(element); removeNextStringTag = true; continue; } + // and remove the first right sibling (this will happen at the next iteration of the loop if ((element.tag === 'string') && removeNextStringTag) { pdict.remove(element); removeNextStringTag = false; } } + + // set any missing required keys to default + for (const key of Object.keys(this.requiredOptionsDefaults)) { + if (existingRequiredKeys.includes(key)) { + continue; + } + setOptions[key] = this.optionsToPlistKeys[key]; + if (!options[key]) { + options[key] = this.requiredOptionsDefaults[key]; + } + } + // add again the new settings for (const [optionKey, plistKey] of Object.entries(setOptions)) { const plistValue = options[optionKey]; @@ -237,7 +277,7 @@ export abstract class DeployConfCommand extends DeployCoreCommand { return false; } if (!stringXmlPath) { - this.env.log.info(`No Capacitor Android project found.`); + this.env.log.warn(`No ${chalk.bold('Capacitor Android')} project found you will need to rerun ${chalk.yellow('ionic deploy configure')} if you add it later.`); return false; } // try to load the plist file first @@ -289,6 +329,23 @@ export abstract class DeployConfCommand extends DeployCoreCommand { element.text = options[optionKey] as string; } } + + // make sure required keys are set + // TODO + for (const [stringKey, optionKey] of Object.entries(this.requiredOptionsFromXmlVal)) { + let element = root.find(`./string[@name="${stringKey}"]`); + // if the tag already exists, just update the content + if (element) { + continue; + } else { + // otherwise create the tag and set to default + element = et.SubElement(root, 'string'); + element.set('name', stringKey); + console.log(optionKey, 'opoitn key'); + element.text = this.requiredOptionsDefaults[optionKey]; + } + } + // write back the modified plist const newXML = etree.write({ encoding: 'utf-8', From 36f5716e9e0d322a89e8161b6b2fb129b1154bc7 Mon Sep 17 00:00:00 2001 From: Nicholas Hyatt Date: Wed, 27 Jan 2021 13:13:02 -0600 Subject: [PATCH 2/3] remove TODO comment --- packages/@ionic/cli/src/commands/deploy/core.ts | 1 - 1 file changed, 1 deletion(-) diff --git a/packages/@ionic/cli/src/commands/deploy/core.ts b/packages/@ionic/cli/src/commands/deploy/core.ts index 6da7dc3f5e..a1cf9b82a2 100644 --- a/packages/@ionic/cli/src/commands/deploy/core.ts +++ b/packages/@ionic/cli/src/commands/deploy/core.ts @@ -331,7 +331,6 @@ export abstract class DeployConfCommand extends DeployCoreCommand { } // make sure required keys are set - // TODO for (const [stringKey, optionKey] of Object.entries(this.requiredOptionsFromXmlVal)) { let element = root.find(`./string[@name="${stringKey}"]`); // if the tag already exists, just update the content From c10c3fedca47612b0d982e53bdf58075c62c53c7 Mon Sep 17 00:00:00 2001 From: Giovanni Di Milia Date: Wed, 27 Jan 2021 15:16:30 -0500 Subject: [PATCH 3/3] Remove chalk and use semantically named colors --- packages/@ionic/cli/src/commands/deploy/add.ts | 2 +- packages/@ionic/cli/src/commands/deploy/core.ts | 10 +++++++--- 2 files changed, 8 insertions(+), 4 deletions(-) diff --git a/packages/@ionic/cli/src/commands/deploy/add.ts b/packages/@ionic/cli/src/commands/deploy/add.ts index 52c1e25879..ae2b7ec4a5 100644 --- a/packages/@ionic/cli/src/commands/deploy/add.ts +++ b/packages/@ionic/cli/src/commands/deploy/add.ts @@ -126,7 +126,7 @@ For Cordova projects it just takes care of running the proper Cordova CLI comman // check if it is already installed const alreadyAdded = await this.checkDeployInstalled(); if (!alreadyAdded) { - this.addPlugin(options, runinfo, integration); + await this.addPlugin(options, runinfo, integration); } else { this.env.log.warn("Live Updates plugin already added. Reconfiguring only."); } diff --git a/packages/@ionic/cli/src/commands/deploy/core.ts b/packages/@ionic/cli/src/commands/deploy/core.ts index a1cf9b82a2..38c4b1683f 100644 --- a/packages/@ionic/cli/src/commands/deploy/core.ts +++ b/packages/@ionic/cli/src/commands/deploy/core.ts @@ -2,7 +2,6 @@ import { CommandLineOptions, combine, contains, validators } from '@ionic/cli-fr import { pathExists, pathWritable, readFile, writeFile } from '@ionic/utils-fs'; import * as et from 'elementtree'; import * as path from 'path'; -import * as chalk from 'chalk'; import { input, strong } from '../../lib/color'; import { Command } from '../../lib/command'; @@ -151,7 +150,10 @@ export abstract class DeployConfCommand extends DeployCoreCommand { return false; } if (!plistPath) { - this.env.log.warn(`No ${chalk.bold('Capacitor iOS')} project found you will need to rerun ${chalk.yellow('ionic deploy configure')} if you add it later.`); + this.env.log.warn( + `No ${strong('Capacitor iOS')} project found\n` + + `You will need to rerun ${input('ionic deploy configure')} if you add it later.\n` + ); return false; } // try to load the plist file first @@ -277,7 +279,9 @@ export abstract class DeployConfCommand extends DeployCoreCommand { return false; } if (!stringXmlPath) { - this.env.log.warn(`No ${chalk.bold('Capacitor Android')} project found you will need to rerun ${chalk.yellow('ionic deploy configure')} if you add it later.`); + this.env.log.warn( + `No ${strong('Capacitor Android')} project found\n`+ + `You will need to rerun ${input('ionic deploy configure')} if you add it later.\n`); return false; } // try to load the plist file first