diff --git a/src/commands/app/build.js b/src/commands/app/build.js index 3e07c0d9..ad5fa4d3 100644 --- a/src/commands/app/build.js +++ b/src/commands/app/build.js @@ -15,7 +15,7 @@ const chalk = require('chalk') const BaseCommand = require('../../BaseCommand') const { Flags } = require('@oclif/core') -const { runInProcess, writeConfig } = require('../../lib/app-helper') +const { runInProcess, writeConfig, reloadEnvOnHooks } = require('../../lib/app-helper') const RuntimeLib = require('@adobe/aio-lib-runtime') const { bundle } = require('@adobe/aio-lib-web') const fs = require('fs-extra') @@ -68,6 +68,7 @@ class Build extends BaseCommand { } catch (err) { this.log(err) } + reloadEnvOnHooks() if (flags.actions) { // removed flags['force-build'] || as it is always forced at this point, we need to check to decide what to build @@ -117,7 +118,7 @@ class Build extends BaseCommand { const bundleOptions = { shouldDisableCache: true, shouldContentHash: flags['content-hash'], - shouldOptimize: flags['web-optimize'], + shouldOptimize: flags['web-optimize'] ?? process.env.NODE_ENV === 'production', logLevel: flags.verbose ? 'verbose' : 'warn' } // empty the dist folder to prevent an S3 explosion @@ -179,8 +180,8 @@ Build.flags = { allowNo: true }), 'web-optimize': Flags.boolean({ - description: '[default: false] Enable optimization (minification) of js/css/html', - default: false + description: '[default: true if NODE_ENV=production, false otherwise] Enable optimization (minification) of js/css/html', + allowNo: true }), extension: Flags.string({ description: 'Build only a specific extension point, the flags can be specified multiple times', diff --git a/src/commands/app/deploy.js b/src/commands/app/deploy.js index bc275c4d..dfb7efc9 100644 --- a/src/commands/app/deploy.js +++ b/src/commands/app/deploy.js @@ -19,7 +19,7 @@ const BuildCommand = require('./build') const webLib = require('@adobe/aio-lib-web') const { Flags } = require('@oclif/core') const { - rewriteActionUrlInEntities, runInProcess, + rewriteActionUrlInEntities, runInProcess, reloadEnvOnHooks, buildExtensionPointPayloadWoMetadata, buildExcShellViewExtensionMetadata, getFilesCountWithExtension } = require('../../lib/app-helper') @@ -206,6 +206,7 @@ class Deploy extends BuildCommand { } catch (err) { this.error(err) } + reloadEnvOnHooks() // provision database if configured if (config.manifest?.full?.database?.['auto-provision'] === true) { @@ -487,8 +488,8 @@ Deploy.flags = { exclusive: ['action', 'publish'] // no-publish is excluded }), 'web-optimize': Flags.boolean({ - description: '[default: false] Enable optimization (minification) of web js/css/html', - default: false + description: '[default: true if NODE_ENV=production, false otherwise] Enable optimization (minification) of web js/css/html', + allowNo: true }), 'log-forwarding-update': Flags.boolean({ description: '[default: true] Update log forwarding configuration on server', diff --git a/src/commands/app/run.js b/src/commands/app/run.js index bc5c907e..ffe809c9 100644 --- a/src/commands/app/run.js +++ b/src/commands/app/run.js @@ -23,7 +23,7 @@ const coreConfig = require('@adobe/aio-lib-core-config') const BaseCommand = require('../../BaseCommand') const runDev = require('../../lib/run-dev') const { defaultHttpServerPort: SERVER_DEFAULT_PORT } = require('../../lib/defaults') -const { runInProcess } = require('../../lib/app-helper') +const { runInProcess, reloadEnvOnHooks } = require('../../lib/app-helper') const DEV_KEYS_DIR = 'dist/dev-keys/' const PRIVATE_KEY_PATH = DEV_KEYS_DIR + 'private.key' @@ -84,6 +84,7 @@ class Run extends BaseCommand { } catch (err) { this.log(err) } + reloadEnvOnHooks() // check if there are certificates available, and generate them if not ... // only care about certificates if the application has a UI diff --git a/src/commands/app/undeploy.js b/src/commands/app/undeploy.js index 743af8be..206ee65b 100644 --- a/src/commands/app/undeploy.js +++ b/src/commands/app/undeploy.js @@ -17,7 +17,7 @@ const { Flags } = require('@oclif/core') const BaseCommand = require('../../BaseCommand') const webLib = require('@adobe/aio-lib-web') -const { runInProcess, buildExtensionPointPayloadWoMetadata } = require('../../lib/app-helper') +const { runInProcess, buildExtensionPointPayloadWoMetadata, reloadEnvOnHooks } = require('../../lib/app-helper') const rtLib = require('@adobe/aio-lib-runtime') const { sendAppAssetsUndeployedAuditLog, sendAppUndeployAuditLog } = require('../../lib/audit-logger') const { setRuntimeApiHostAndAuthHandler, getAccessToken } = require('../../lib/auth-helper') @@ -136,6 +136,7 @@ class Undeploy extends BaseCommand { } catch (err) { this.log(err) } + reloadEnvOnHooks() if (flags.actions) { if (config.app.hasBackend) { diff --git a/src/lib/app-helper.js b/src/lib/app-helper.js index ae3bebc2..a423d40a 100644 --- a/src/lib/app-helper.js +++ b/src/lib/app-helper.js @@ -81,6 +81,23 @@ async function runPackageScript (scriptName, dir, cmdArgs = []) { } } +/** + * Reloads the aio config so that changes made by a hook to .env or .aio are picked up. + * Opt-in: it only reloads when the AIO_RELOAD_ENV_ON_HOOKS env var is set to a truthy value. + * + * @returns {boolean} true if the config was reloaded + */ +function reloadEnvOnHooks () { + const value = (process.env.AIO_RELOAD_ENV_ON_HOOKS || '').trim().toLowerCase() + if (value === '' || value === '0' || value === 'false') { + aioLogger.debug('reloadEnvOnHooks: AIO_RELOAD_ENV_ON_HOOKS is not set, skipping config reload') + return false + } + aioLogger.debug('reloadEnvOnHooks: AIO_RELOAD_ENV_ON_HOOKS is set, reloading config') + aioConfig.reload() + return true +} + /** * @param {string} hookPath to be require()'d and run. Should export an async function that takes a config object as its only argument * @param {object} config which will be passed to the hook @@ -557,6 +574,7 @@ module.exports = { installPackages, runScript, runInProcess, + reloadEnvOnHooks, runPackageScript, wrapError, removeProtocolFromURL, diff --git a/src/lib/build-actions.js b/src/lib/build-actions.js index 547b84cd..428aa6d4 100644 --- a/src/lib/build-actions.js +++ b/src/lib/build-actions.js @@ -10,7 +10,7 @@ OF ANY KIND, either express or implied. See the License for the specific languag governing permissions and limitations under the License. */ -const { runInProcess } = require('./app-helper') +const { runInProcess, reloadEnvOnHooks } = require('./app-helper') const { buildActions } = require('@adobe/aio-lib-runtime') /** @@ -22,6 +22,7 @@ const { buildActions } = require('@adobe/aio-lib-runtime') */ module.exports = async (config, filterActions, forceBuild = false) => { await runInProcess(config.hooks['pre-app-build'], config) + reloadEnvOnHooks() const script = await runInProcess(config.hooks['build-actions'], { config, options: { filterActions, forceBuild } }) if (!script) { await buildActions(config, filterActions, forceBuild) diff --git a/src/lib/deploy-actions.js b/src/lib/deploy-actions.js index fca533b5..75d9fcaa 100644 --- a/src/lib/deploy-actions.js +++ b/src/lib/deploy-actions.js @@ -10,7 +10,7 @@ OF ANY KIND, either express or implied. See the License for the specific languag governing permissions and limitations under the License. */ -const { runInProcess } = require('./app-helper') +const { runInProcess, reloadEnvOnHooks } = require('./app-helper') const { deployActions } = require('@adobe/aio-lib-runtime') const logActions = require('./log-actions') @@ -31,6 +31,7 @@ module.exports = async ({ inprocHook }) => { await runInProcess(config.hooks['pre-app-deploy'], config) + reloadEnvOnHooks() const hookFilterEntities = Array.isArray(deployConfig.filterEntities?.actions) ? deployConfig.filterEntities.actions : [] const hookData = { diff --git a/test/commands/app/build.test.js b/test/commands/app/build.test.js index d6ad110d..f35ba307 100644 --- a/test/commands/app/build.test.js +++ b/test/commands/app/build.test.js @@ -255,6 +255,37 @@ describe('run', () => { expect(mockBundleFunc).toHaveBeenCalledTimes(1) }) + test('reloads env after pre-app-build and before building', async () => { + helpers.reloadEnvOnHooks.mockClear() + command.getAppExtConfigs.mockResolvedValueOnce(createAppConfig(command.appConfig)) + + await command.run() + expect(helpers.reloadEnvOnHooks).toHaveBeenCalledTimes(1) + expect(helpers.runInProcess.mock.invocationCallOrder[0]).toBeLessThan(helpers.reloadEnvOnHooks.mock.invocationCallOrder[0]) + expect(helpers.reloadEnvOnHooks.mock.invocationCallOrder[0]).toBeLessThan(mockRuntimeLib.buildActions.mock.invocationCallOrder[0]) + }) + + test.each([ + [[], 'production', true], + [[], 'development', false], + [[], undefined, false], + [['--web-optimize'], undefined, true], + [['--no-web-optimize'], 'production', false] + ])('web-optimize %j with NODE_ENV=%s optimizes: %s', async (argv, nodeEnv, expected) => { + const originalNodeEnv = process.env.NODE_ENV + if (nodeEnv) process.env.NODE_ENV = nodeEnv + else delete process.env.NODE_ENV + try { + command.getAppExtConfigs.mockResolvedValueOnce(createAppConfig(command.appConfig)) + command.argv = argv + await command.run() + expect(mockWebLib.bundle).toHaveBeenCalledWith(expect.any(String), expect.any(String), + expect.objectContaining({ shouldOptimize: expected }), expect.any(Function)) + } finally { + process.env.NODE_ENV = originalNodeEnv + } + }) + test('build & deploy an App with --no-content-hash', async () => { command.getAppExtConfigs.mockResolvedValueOnce(createAppConfig(command.appConfig)) diff --git a/test/commands/app/deploy.test.js b/test/commands/app/deploy.test.js index 40bb0d97..dd9f842c 100644 --- a/test/commands/app/deploy.test.js +++ b/test/commands/app/deploy.test.js @@ -869,8 +869,9 @@ describe('run', () => { expect(command.error).toHaveBeenCalledTimes(0) }) - test('deploy reloads config before deploying actions', async () => { + test('deploy reloads env after pre-app-deploy and reloads config before deploying actions', async () => { command.getAppExtConfigs.mockResolvedValueOnce(createAppConfig(command.appConfig)) + helpers.reloadEnvOnHooks.mockClear() const noScriptFound = undefined helpers.runInProcess .mockResolvedValueOnce(noScriptFound) // pre-app-deploy @@ -880,8 +881,13 @@ describe('run', () => { command.argv = ['--no-web-assets'] await command.run() - expect(mockConfig.reload).toHaveBeenCalledTimes(1) expect(mockRuntimeLib.deployActions).toHaveBeenCalledTimes(1) + // env reload is gated, the config reload just before deploying actions is not + expect(helpers.reloadEnvOnHooks).toHaveBeenCalledTimes(1) + expect(helpers.runInProcess.mock.invocationCallOrder[0]).toBeLessThan( + helpers.reloadEnvOnHooks.mock.invocationCallOrder[0] + ) + expect(mockConfig.reload).toHaveBeenCalledTimes(1) expect(mockConfig.reload.mock.invocationCallOrder[0]).toBeLessThan( mockRuntimeLib.deployActions.mock.invocationCallOrder[0] ) @@ -1368,8 +1374,7 @@ describe('run', () => { 'log-forwarding-update': true, open: false, publish: false, - 'web-assets': true, - 'web-optimize': false + 'web-assets': true }, env: mockEnv, opItems: [ @@ -1492,8 +1497,7 @@ describe('run', () => { 'log-forwarding-update': true, open: false, publish: false, - 'web-assets': true, - 'web-optimize': false + 'web-assets': true }, env: mockEnv, opItems: [ diff --git a/test/commands/app/run.test.js b/test/commands/app/run.test.js index 1cf3784f..07c05585 100644 --- a/test/commands/app/run.test.js +++ b/test/commands/app/run.test.js @@ -701,4 +701,22 @@ describe('run', () => { expect(scriptSequence[0]).toEqual('pre-app-run') expect(scriptSequence[1]).toEqual('post-app-run') }) + + test('reloads env after pre-app-run and before starting the dev environment', async () => { + mockFSExists([PRIVATE_KEY_PATH, PUB_CERT_PATH]) + helpers.reloadEnvOnHooks.mockClear() + command.getAppExtConfigs.mockResolvedValueOnce(createAppConfig(command.appConfig)) + + command.argv = [] + await command.run() + + expect(helpers.reloadEnvOnHooks).toHaveBeenCalledTimes(1) + expect(mockRunDev).toHaveBeenCalledTimes(1) + expect(helpers.runInProcess.mock.invocationCallOrder[0]).toBeLessThan( + helpers.reloadEnvOnHooks.mock.invocationCallOrder[0] + ) + expect(helpers.reloadEnvOnHooks.mock.invocationCallOrder[0]).toBeLessThan( + mockRunDev.mock.invocationCallOrder[0] + ) + }) }) diff --git a/test/commands/app/undeploy.test.js b/test/commands/app/undeploy.test.js index 788d1f6f..bf230dfa 100644 --- a/test/commands/app/undeploy.test.js +++ b/test/commands/app/undeploy.test.js @@ -198,6 +198,16 @@ describe('run', () => { expect(mockWebLib.undeployWeb).toHaveBeenCalledTimes(1) }) + test('reloads env after pre-app-undeploy and before undeploying', async () => { + helpers.reloadEnvOnHooks.mockClear() + command.getAppExtConfigs.mockResolvedValueOnce(createAppConfig()) + + await command.run() + expect(helpers.reloadEnvOnHooks).toHaveBeenCalledTimes(1) + expect(helpers.runInProcess.mock.invocationCallOrder[0]).toBeLessThan(helpers.reloadEnvOnHooks.mock.invocationCallOrder[0]) + expect(helpers.reloadEnvOnHooks.mock.invocationCallOrder[0]).toBeLessThan(mockRuntimeLib.undeployActions.mock.invocationCallOrder[0]) + }) + test('undeploy an App with no flags with hooks', async () => { command.getAppExtConfigs.mockResolvedValueOnce(createAppConfig()) __setupMockHooks() diff --git a/test/commands/lib/app-helper.test.js b/test/commands/lib/app-helper.test.js index 28ed4655..ffa97bf6 100644 --- a/test/commands/lib/app-helper.test.js +++ b/test/commands/lib/app-helper.test.js @@ -42,6 +42,7 @@ beforeEach(() => { execa.command.mockReset() aioConfig.get.mockReset() aioConfig.set.mockReset() + aioConfig.reload.mockReset() libEnv.getCliEnv.mockReset() libIms.getToken.mockReset() mockFetch.mockReset() @@ -248,6 +249,36 @@ test('runInProcess fails with no hook-path', async () => { expect(mockLogger.debug).toHaveBeenCalledWith('runInProcess: undefined hookPath') }) +describe('reloadEnvOnHooks', () => { + const OLD_VALUE = process.env.AIO_RELOAD_ENV_ON_HOOKS + + afterEach(() => { + if (OLD_VALUE === undefined) { + delete process.env.AIO_RELOAD_ENV_ON_HOOKS + } else { + process.env.AIO_RELOAD_ENV_ON_HOOKS = OLD_VALUE + } + }) + + test('does not reload when AIO_RELOAD_ENV_ON_HOOKS is not set', () => { + delete process.env.AIO_RELOAD_ENV_ON_HOOKS + expect(appHelper.reloadEnvOnHooks()).toBe(false) + expect(aioConfig.reload).not.toHaveBeenCalled() + }) + + test.each(['', ' ', '0', 'false', 'FALSE'])('does not reload when AIO_RELOAD_ENV_ON_HOOKS is %p', (value) => { + process.env.AIO_RELOAD_ENV_ON_HOOKS = value + expect(appHelper.reloadEnvOnHooks()).toBe(false) + expect(aioConfig.reload).not.toHaveBeenCalled() + }) + + test.each(['1', 'true', 'TRUE', 'yes'])('reloads when AIO_RELOAD_ENV_ON_HOOKS is %p', (value) => { + process.env.AIO_RELOAD_ENV_ON_HOOKS = value + expect(appHelper.reloadEnvOnHooks()).toBe(true) + expect(aioConfig.reload).toHaveBeenCalledTimes(1) + }) +}) + test('runScript with empty command', async () => { await appHelper.runScript(undefined, 'dir') expect(execa.command).not.toHaveBeenCalled() diff --git a/test/commands/lib/build-actions.test.js b/test/commands/lib/build-actions.test.js index a4fa171e..d64ba69e 100644 --- a/test/commands/lib/build-actions.test.js +++ b/test/commands/lib/build-actions.test.js @@ -28,6 +28,15 @@ beforeEach(() => { utils.runInProcess.mockReset() }) +test('reloads env after pre-app-build', async () => { + utils.reloadEnvOnHooks.mockClear() + utils.runInProcess.mockImplementation(script => script === 'build-actions' ? {} : undefined) + await buildActions(extensionConfig) + expect(utils.reloadEnvOnHooks).toHaveBeenCalledTimes(1) + expect(utils.runInProcess.mock.invocationCallOrder[0]).toBeLessThan(utils.reloadEnvOnHooks.mock.invocationCallOrder[0]) + expect(utils.reloadEnvOnHooks.mock.invocationCallOrder[0]).toBeLessThan(utils.runInProcess.mock.invocationCallOrder[1]) +}) + test('exports', () => { expect(typeof buildActions).toEqual('function') }) diff --git a/test/commands/lib/deploy-actions.test.js b/test/commands/lib/deploy-actions.test.js index e1ad539c..2457388e 100644 --- a/test/commands/lib/deploy-actions.test.js +++ b/test/commands/lib/deploy-actions.test.js @@ -35,6 +35,14 @@ test('exports', () => { expect(typeof deployActions).toEqual('function') }) +test('reloads env after pre-app-deploy', async () => { + utils.reloadEnvOnHooks.mockClear() + await deployActions({ config: { hooks: {} } }) + expect(utils.reloadEnvOnHooks).toHaveBeenCalledTimes(1) + expect(utils.runInProcess.mock.invocationCallOrder[0]).toBeLessThan(utils.reloadEnvOnHooks.mock.invocationCallOrder[0]) + expect(utils.reloadEnvOnHooks.mock.invocationCallOrder[0]).toBeLessThan(rtDeployActions.mock.invocationCallOrder[0]) +}) + test('deploy-actions app hook available', async () => { utils.runInProcess.mockImplementation(script => { if (script === 'deploy-actions') {