diff --git a/package.json b/package.json index c934e774..1b5861e2 100644 --- a/package.json +++ b/package.json @@ -12,7 +12,7 @@ "@adobe/aio-lib-core-networking": "^5", "@adobe/aio-lib-env": "^3", "@adobe/aio-lib-ims": "^7", - "@adobe/aio-lib-runtime": "^6", + "@adobe/aio-lib-runtime": "^7.0.0", "@adobe/aio-lib-templates": "^3", "@adobe/aio-lib-web": "^7", "@adobe/generator-aio-app": "^7", diff --git a/src/commands/app/build.js b/src/commands/app/build.js index 2dcba6f6..c0c04707 100644 --- a/src/commands/app/build.js +++ b/src/commands/app/build.js @@ -70,14 +70,16 @@ class Build extends BaseCommand { } if (flags.actions) { - if (config.app.hasBackend && (flags['force-build'] || !fs.existsSync(config.actions.dist))) { + // removed flags['force-build'] || as it is always forced at this point, we need to check to decide what to build + // if no backend, we skip the build + if (config.app.hasBackend) { try { let builtList = [] const script = await runInProcess(config.hooks['build-actions'], config) aioLogger.debug(`run hook for 'build-actions' for actions in '${name}' returned ${script}`) spinner.start(`Building actions for '${name}'`) if (!script) { - builtList = await RuntimeLib.buildActions(config, filterActions, true) + builtList = await RuntimeLib.buildActions(config, filterActions, flags['force-build']) // skipCheck is false } if (builtList.length > 0) { spinner.succeed(chalk.green(`Built ${builtList.length} action(s) for '${name}'`)) @@ -85,7 +87,7 @@ class Build extends BaseCommand { if (script) { spinner.fail(chalk.green(`build-action skipped by hook '${name}'`)) } else { - spinner.fail(chalk.green(`No actions built for '${name}'`)) + spinner.info(chalk.green(`No actions built for '${name}'`)) } } aioLogger.debug(`RuntimeLib.buildActions returned ${builtList}`) @@ -94,7 +96,7 @@ class Build extends BaseCommand { throw err } } else { - spinner.info(`no backend or a build already exists, skipping action build for '${name}'`) + spinner.info(`no backend, skipping action build for '${name}'`) } } if (flags['web-assets']) { @@ -127,7 +129,8 @@ class Build extends BaseCommand { throw err } } else { - spinner.info(`no frontend or a build already exists, skipping frontend build for '${name}'`) + // TODO: specify which ... keep this useful + spinner.info(chalk.green(`No frontend or a build already exists, skipping frontend build for '${name}'`)) } } try { @@ -140,7 +143,8 @@ class Build extends BaseCommand { Build.description = `Build an Adobe I/O App -This will always force a rebuild unless --no-force-build is set. +Build the actions and web assets for an Adobe I/O App. Build is optimized to only build what is necessary. +Use the --force-build flag to force a build even if one already exists. ` Build.flags = { @@ -163,8 +167,8 @@ Build.flags = { allowNo: true }), 'force-build': Flags.boolean({ - description: '[default: true] Force a build even if one already exists', - default: true, + description: '[default: false] Force a build even if one already exists', + default: false, allowNo: true }), 'content-hash': Flags.boolean({ diff --git a/src/commands/app/deploy.js b/src/commands/app/deploy.js index 7094a9c2..ac495500 100644 --- a/src/commands/app/deploy.js +++ b/src/commands/app/deploy.js @@ -172,7 +172,7 @@ class Deploy extends BuildCommand { // output should be "Error : : \n" for each failure this.error(hookResults.failures.map(f => `${f.plugin.name} : ${f.error.message}`).join('\nError: '), { exit: 1 }) } - deployedRuntimeEntities = await rtLib.deployActions(config, { filterEntities }, onProgress) + deployedRuntimeEntities = await rtLib.deployActions(config, { filterEntities, useForce: flags['force-deploy'] }, onProgress) } if (deployedRuntimeEntities.actions && deployedRuntimeEntities.actions.length > 0) { @@ -181,7 +181,7 @@ class Deploy extends BuildCommand { if (script) { spinner.fail(chalk.green(`deploy-actions skipped by hook '${name}'`)) } else { - spinner.fail(chalk.green(`No actions deployed for '${name}'`)) + spinner.info(chalk.green(`No actions deployed for '${name}'`)) } } } catch (err) { @@ -287,9 +287,13 @@ class Deploy extends BuildCommand { } } -Deploy.description = `Build and deploy an Adobe I/O App +Deploy.description = `Deploy an Adobe I/O App -This will always force a rebuild unless --no-force-build is set. +Deploys the actions and web assets for an Adobe I/O App. +This will also build any changed actions or web assets before deploying. +Use the --force-build flag to force a build even if one already exists. +Deploy is optimized to only deploy what is necessary. Be aware that deploying actions will overwrite any previous deployments. +Use the --force-deploy flag to force deploy changes, regardless of production Workspace being published in Exchange. ` Deploy.flags = { @@ -319,7 +323,7 @@ Deploy.flags = { 'force-build': Flags.boolean({ description: '[default: true] Force a build even if one already exists', exclusive: ['no-build'], // no-build - default: true, + default: false, allowNo: true }), 'content-hash': Flags.boolean({ diff --git a/src/commands/app/run.js b/src/commands/app/run.js index 3ed611bf..f48c65cb 100644 --- a/src/commands/app/run.js +++ b/src/commands/app/run.js @@ -108,6 +108,9 @@ class Run extends BaseCommand { } const verboseOutput = flags.verbose || flags.local || headlessApp + // we should evaluate this, a lot of output just disappears in the spinner text and + // using verbose dumps ALL of parcel's output, so this become unreadable + // we need a middle ground. -jm const onProgress = !verboseOutput ? info => { spinner.text = info diff --git a/src/lib/actions-watcher.js b/src/lib/actions-watcher.js index 135891ff..d92be398 100644 --- a/src/lib/actions-watcher.js +++ b/src/lib/actions-watcher.js @@ -38,7 +38,7 @@ const deployActions = require('./deploy-actions') module.exports = async (watcherOptions) => { const { config, log } = watcherOptions - log(`watching action files at ${config.actions.src}...`) + log(`watching action files at ${config.actions.src} ...`) const watcher = chokidar.watch(config.actions.src) watcher.on('change', createChangeHandler({ ...watcherOptions, watcher })) @@ -91,6 +91,8 @@ function createChangeHandler (watcherOptions) { try { aioLogger.debug(`${filePath} has changed. Redeploying actions.`) const filterActions = getActionNameFromPath(filePath, watcherOptions) + // this is happening, but its not showing up because verbose is usually off + // this might be more important and worthy of signalling to the user if (!filterActions.length) { log(' -> A non-action file was changed, restart is required to deploy...') } else { @@ -119,6 +121,13 @@ function createChangeHandler (watcherOptions) { * @returns {Array} All of the actions which match the modified path */ function getActionNameFromPath (filePath, watcherOptions) { + // note: this check only happens during aio app run + // before the watcher is started, all actions are built and deployed and hashes updated + // this code is missing 2 cases: + // 1. if the action is a folder with a package.json and the changed file is in the folder + // 2. if the changed file is in the folder with the action, but not the action file itself + // we need to be careful with these cases, because we could cause a recursive loop + // for now we continue to error on the cautious side, and output a message suggesting a restart const actionNames = [] const unixFilePath = upath.toUnix(filePath) const { config } = watcherOptions @@ -126,7 +135,10 @@ function getActionNameFromPath (filePath, watcherOptions) { if (pkg.actions) { Object.entries(pkg.actions).forEach(([actionName, action]) => { const unixActionFunction = upath.toUnix(action.function) - if (unixActionFunction.includes(unixFilePath)) { + // since action could be a folder, and changed file could be in the folder + // we need to compare both ways + // there are 2 additional cases listed above + if (unixActionFunction.includes(unixFilePath) || unixFilePath.includes(unixActionFunction)) { actionNames.push(actionName) } }) diff --git a/src/lib/run-dev.js b/src/lib/run-dev.js index e4e3aec2..9c02130f 100644 --- a/src/lib/run-dev.js +++ b/src/lib/run-dev.js @@ -82,8 +82,7 @@ async function runDev (config, dataDir, options = {}, log = () => {}, inprocHook // build and deploy actions log('building actions..') - await buildActions(devConfig, null, true /* force build */) - + await buildActions(devConfig, null, false /* force build */) const { cleanup: watcherCleanup } = await actionsWatcher({ config: devConfig, isLocal, log, inprocHook }) cleanup.add(() => watcherCleanup(), 'stopping action watcher...') } @@ -152,10 +151,16 @@ async function runDev (config, dataDir, options = {}, log = () => {}, inprocHook devConfig.app.hasFrontend = false } - log('setting up vscode debug configuration files...') - const vscodeConfig = vscode(devConfig) - await vscodeConfig.update({ frontEndUrl }) - cleanup.add(() => vscodeConfig.cleanup(), 'cleaning up vscode debug configuration files...') + // todo: remove vscode config swapping, dev command uses a persistent file so we don't need this. + // also there was a latent issue with projects that defined an action src as a folder with an index.js file. + // it looks explicitly for package.json and fails if it does not find it. + // regarless, we don't need it, and when we actually remove --local we can be rid of this. + if (isLocal) { + log('setting up vscode debug configuration files...') + const vscodeConfig = vscode(devConfig) + await vscodeConfig.update({ frontEndUrl }) + cleanup.add(() => vscodeConfig.cleanup(), 'cleaning up vscode debug configuration files...') + } // automatically fetch logs if there are actions if (config.app.hasBackend && fetchLogs) { diff --git a/test/commands/app/build.test.js b/test/commands/app/build.test.js index 79576590..fae15862 100644 --- a/test/commands/app/build.test.js +++ b/test/commands/app/build.test.js @@ -182,7 +182,7 @@ test('flags', async () => { expect(typeof TheCommand.flags['force-build']).toBe('object') expect(typeof TheCommand.flags['force-build'].description).toBe('string') - expect(TheCommand.flags['force-build'].default).toEqual(true) + expect(TheCommand.flags['force-build'].default).toEqual(false) expect(TheCommand.flags['force-build'].allowNo).toEqual(true) expect(typeof TheCommand.flags['content-hash']).toBe('object') @@ -285,6 +285,7 @@ describe('run', () => { }) test('build & deploy an App with no force-build but build exists', async () => { + // build actions is now called so it can verify if the src has changed since the last build command.appConfig.actions.dist = 'actions' command.appConfig.web.distProd = 'dist' command.getAppExtConfigs.mockResolvedValueOnce(createAppConfig(command.appConfig)) @@ -293,7 +294,7 @@ describe('run', () => { mockFS.existsSync.mockReturnValue(true) await command.run() expect(command.error).toHaveBeenCalledTimes(0) - expect(mockRuntimeLib.buildActions).toHaveBeenCalledTimes(0) + expect(mockRuntimeLib.buildActions).toHaveBeenCalledTimes(1) expect(mockWebLib.bundle).toHaveBeenCalledTimes(0) }) @@ -326,7 +327,7 @@ describe('run', () => { expect(spinner.succeed).toHaveBeenCalledWith(expect.stringContaining('Built 3 action(s) for \'application\'')) expect(command.error).toHaveBeenCalledTimes(0) expect(mockRuntimeLib.buildActions).toHaveBeenCalledTimes(1) - expect(mockRuntimeLib.buildActions).toHaveBeenCalledWith(appConfig.application, ['a', 'b', 'c'], true) + expect(mockRuntimeLib.buildActions).toHaveBeenCalledWith(appConfig.application, ['a', 'b', 'c'], false) expect(mockWebLib.bundle).toHaveBeenCalledTimes(0) }) @@ -344,9 +345,9 @@ describe('run', () => { test('build & deploy with --no-actions', async () => { command.getAppExtConfigs.mockResolvedValueOnce(createAppConfig(command.appConfig)) - - command.argv = ['--no-actions'] + command.argv = ['--no-actions', '--force-build'] mockFS.existsSync.mockReturnValue(true) + await command.run() expect(command.error).toHaveBeenCalledTimes(0) expect(mockWebLib.bundle).toHaveBeenCalledTimes(1) diff --git a/test/commands/app/deploy.test.js b/test/commands/app/deploy.test.js index efa8288b..8ea2574c 100644 --- a/test/commands/app/deploy.test.js +++ b/test/commands/app/deploy.test.js @@ -194,7 +194,7 @@ test('flags', async () => { expect(typeof TheCommand.flags['force-build']).toBe('object') expect(typeof TheCommand.flags['force-build'].description).toBe('string') - expect(TheCommand.flags['force-build'].default).toEqual(true) + expect(TheCommand.flags['force-build'].default).toEqual(false) expect(TheCommand.flags['force-build'].allowNo).toEqual(true) expect(typeof TheCommand.flags['content-hash']).toBe('object') @@ -293,7 +293,10 @@ describe('run', () => { expect(mockRuntimeLib.deployActions).toHaveBeenCalledTimes(1) expect(mockWebLib.deployWeb).toHaveBeenCalledTimes(1) expect(command.buildOneExt).toHaveBeenCalledTimes(1) - expect(command.buildOneExt).toHaveBeenCalledWith('application', appConfig.application, expect.objectContaining({ 'force-build': true, verbose: true }), expect.anything()) + expect(command.buildOneExt).toHaveBeenCalledWith('application', + appConfig.application, + expect.objectContaining({ 'force-build': false, verbose: true }), + expect.anything()) }) test('build & deploy --no-web-assets', async () => { @@ -306,7 +309,10 @@ describe('run', () => { expect(mockRuntimeLib.deployActions).toHaveBeenCalledTimes(1) expect(mockWebLib.deployWeb).toHaveBeenCalledTimes(0) expect(command.buildOneExt).toHaveBeenCalledTimes(1) - expect(command.buildOneExt).toHaveBeenCalledWith('application', appConfig.application, expect.objectContaining({ 'force-build': true, 'web-assets': false }), expect.anything()) + expect(command.buildOneExt).toHaveBeenCalledWith('application', + appConfig.application, + expect.objectContaining({ 'force-build': false, 'web-assets': false }), + expect.anything()) }) test('build & deploy only one action using --action (workspace: Production)', async () => { @@ -347,9 +353,13 @@ describe('run', () => { expect(command.buildOneExt).toHaveBeenCalledTimes(1) expect(mockLibConsoleCLI.getApplicationExtensions).toHaveBeenCalledTimes(0) - expect(command.buildOneExt).toHaveBeenCalledWith('application', appConfig.application, expect.objectContaining({ 'force-build': true, 'web-assets': false, action: ['a', 'b', 'c'] }), expect.anything()) + expect(command.buildOneExt).toHaveBeenCalledWith('application', + appConfig.application, + expect.objectContaining({ 'force-build': false, 'web-assets': false, action: ['a', 'b', 'c'] }), + expect.anything()) expect(mockRuntimeLib.deployActions).toHaveBeenCalledWith(appConfig.application, { - filterEntities: { actions: ['a', 'b', 'c'] } + filterEntities: { actions: ['a', 'b', 'c'] }, + useForce: false }, expect.any(Function)) }) @@ -365,9 +375,13 @@ describe('run', () => { expect(mockWebLib.deployWeb).toHaveBeenCalledTimes(0) expect(command.buildOneExt).toHaveBeenCalledTimes(1) - expect(command.buildOneExt).toHaveBeenCalledWith('application', appConfig.application, expect.objectContaining({ 'force-build': true, 'web-assets': false, action: ['c'] }), expect.anything()) + expect(command.buildOneExt).toHaveBeenCalledWith('application', + appConfig.application, + expect.objectContaining({ 'force-build': false, 'web-assets': false, action: ['c'] }), + expect.anything()) expect(mockRuntimeLib.deployActions).toHaveBeenCalledWith(appConfig.application, { - filterEntities: { actions: ['c'] } + filterEntities: { actions: ['c'] }, + useForce: false }, expect.any(Function)) }) @@ -384,7 +398,10 @@ describe('run', () => { expect(mockRuntimeLib.deployActions).toHaveBeenCalledTimes(0) expect(mockWebLib.deployWeb).toHaveBeenCalledTimes(0) expect(command.buildOneExt).toHaveBeenCalledTimes(1) - expect(command.buildOneExt).toHaveBeenCalledWith('application', appConfig.application, expect.objectContaining({ 'force-build': true, 'web-assets': false }), expect.anything()) + expect(command.buildOneExt).toHaveBeenCalledWith('application', + appConfig.application, + expect.objectContaining({ 'force-build': false, 'web-assets': false }), + expect.anything()) }) test('build & deploy actions with no actions folder but with a manifest', async () => { @@ -408,7 +425,10 @@ describe('run', () => { expect(mockRuntimeLib.deployActions).toHaveBeenCalledTimes(0) expect(mockWebLib.deployWeb).toHaveBeenCalledTimes(1) expect(command.buildOneExt).toHaveBeenCalledTimes(1) - expect(command.buildOneExt).toHaveBeenCalledWith('application', appConfig.application, expect.objectContaining({ 'force-build': true, actions: false }), expect.anything()) + expect(command.buildOneExt).toHaveBeenCalledWith('application', + appConfig.application, + expect.objectContaining({ 'force-build': false, actions: false }), + expect.anything()) }) test('build & deploy with --no-actions with no static folder', async () => { @@ -423,7 +443,10 @@ describe('run', () => { expect(mockRuntimeLib.deployActions).toHaveBeenCalledTimes(0) expect(mockWebLib.deployWeb).toHaveBeenCalledTimes(0) expect(command.buildOneExt).toHaveBeenCalledTimes(1) - expect(command.buildOneExt).toHaveBeenCalledWith('application', appConfig.application, expect.objectContaining({ 'force-build': true, actions: false }), expect.anything()) + expect(command.buildOneExt).toHaveBeenCalledWith('application', + appConfig.application, + expect.objectContaining({ 'force-build': false, actions: false }), + expect.anything()) }) test('build & deploy with no manifest.yml', async () => { @@ -437,7 +460,10 @@ describe('run', () => { expect(mockRuntimeLib.deployActions).toHaveBeenCalledTimes(0) expect(mockWebLib.deployWeb).toHaveBeenCalledTimes(1) expect(command.buildOneExt).toHaveBeenCalledTimes(1) - expect(command.buildOneExt).toHaveBeenCalledWith('application', appConfig.application, expect.objectContaining({ 'force-build': true }), expect.anything()) + expect(command.buildOneExt).toHaveBeenCalledWith('application', + appConfig.application, + expect.objectContaining({ 'force-build': false }), + expect.anything()) }) test('--no-build', async () => { @@ -900,6 +926,7 @@ describe('run', () => { expect(mockLibConsoleCLI.getApplicationExtensions).toHaveBeenCalledTimes(1) expect(mockWebLib.deployWeb).toHaveBeenCalledTimes(1) expect(mockRuntimeLib.deployActions).toHaveBeenCalledTimes(1) + expect(mockRuntimeLib.deployActions).toHaveBeenCalledWith(expect.any(Object), expect.objectContaining({ useForce: true }), expect.any(Function)) expect(mockLibConsoleCLI.updateExtensionPoints).toHaveBeenCalledTimes(0) expect(mockLibConsoleCLI.updateExtensionPointsWithoutOverwrites).toHaveBeenCalledTimes(0) }) @@ -928,6 +955,7 @@ describe('run', () => { expect(mockLibConsoleCLI.getApplicationExtensions).toHaveBeenCalledTimes(1) expect(mockWebLib.deployWeb).toHaveBeenCalledTimes(1) expect(mockRuntimeLib.deployActions).toHaveBeenCalledTimes(1) + expect(mockRuntimeLib.deployActions).toHaveBeenCalledWith(expect.any(Object), expect.objectContaining({ useForce: true }), expect.any(Function)) expect(mockLibConsoleCLI.updateExtensionPoints).toHaveBeenCalledTimes(0) expect(mockLibConsoleCLI.updateExtensionPointsWithoutOverwrites).toHaveBeenCalledTimes(0) }) diff --git a/test/commands/lib/run-dev.test.js b/test/commands/lib/run-dev.test.js index e755e489..eff407ba 100644 --- a/test/commands/lib/run-dev.test.js +++ b/test/commands/lib/run-dev.test.js @@ -409,11 +409,13 @@ test('isLocal false, build-static hook set, serve-static hook not set)', async ( expect(actionsWatcher).toHaveBeenCalled() }) -test('runDev always force builds', async () => { +test('runDev does not always force builds', async () => { const config = cloneDeep(createAppConfig().application) await runDev(config, DATA_DIR, { isLocal: false }) expect(buildActions).toHaveBeenCalled() - expect(buildActions).toHaveBeenCalledWith(expect.any(Object) /* config */, null /* filterActions */, true /* forceBuild */) + expect(buildActions).toHaveBeenCalledWith(expect.any(Object) /* config */, + null /* filterActions */, + false /* forceBuild */) })