From 419b9a9a138c526b63d9d0193df789c2dbd55548 Mon Sep 17 00:00:00 2001 From: Robert Jackson Date: Thu, 19 Dec 2019 17:04:21 -0500 Subject: [PATCH] Add `--run-codemod` commandline option. This allows tools like `@ember/octanify` to pass `--run-codemods` in it's own CI system without having to worry about properly mocking `STDIN`. This is not _really_ expected to be used as an actual flag users would specify, but it is fine if they do (which is why the `--run-codemod` flag has a description so that it will show up in `--help` output). --- commands/index.js | 30 ++++++++--- features/application-template-wrapper.js | 42 ++++++++------- features/template-only-glimmer-components.js | 54 +++++++++++--------- tests/commands-test.js | 49 +++++++++++++++++- 4 files changed, 123 insertions(+), 52 deletions(-) diff --git a/commands/index.js b/commands/index.js index ae75cdc..4ba7230 100644 --- a/commands/index.js +++ b/commands/index.js @@ -46,7 +46,7 @@ const SHARED = { return configPath; }, - _setFeature: async function (name, value) { + _setFeature: async function (name, value, shouldRunCodemod) { let feature = FEATURES[name]; if (feature === undefined) { @@ -62,7 +62,7 @@ const SHARED = { } if (typeof feature.callback === 'function') { - await feature.callback(this.project, value); + await feature.callback(this.project, value, shouldRunCodemod); } let config = {}; @@ -134,11 +134,20 @@ const ENABLE_FEATURE = Object.assign({ name: 'feature:enable', description: 'Enable feature.', works: 'insideProject', + availableOptions: [ + { + name: 'run-codemod', + type: Boolean, + description: 'run any associated codemods without prompting' + // intentionally not setting a default, when the value is undefined the + // command will prompt the user + }, + ], anonymousOptions: [ '' ], - run(_, args) { - return this._setFeature(args[0], true); + run(commandOptions, args) { + return this._setFeature(args[0], true, commandOptions.runCodemod); } }, SHARED); @@ -146,11 +155,20 @@ const DISABLE_FEATURE = Object.assign({ name: 'feature:disable', description: 'Disable feature.', works: 'insideProject', + availableOptions: [ + { + name: 'run-codemod', + type: Boolean, + description: 'run any associated codemods without prompting' + // intentionally not setting a default, when the value is undefined the + // command will prompt the user + }, + ], anonymousOptions: [ '' ], - run(_, args) { - return this._setFeature(args[0], false); + run(commandOptions, args) { + return this._setFeature(args[0], false, commandOptions.runCodemod); } }, SHARED); diff --git a/features/application-template-wrapper.js b/features/application-template-wrapper.js index bd4b0af..a22962c 100644 --- a/features/application-template-wrapper.js +++ b/features/application-template-wrapper.js @@ -13,7 +13,7 @@ module.exports = { url: 'https://github.com/emberjs/rfcs/pull/280', default: true, since: '3.1.0', - callback: async function (project, value) { + callback: async function (project, value, shouldRunCodemod) { if (value !== false) { return; } @@ -44,34 +44,38 @@ module.exports = { } } - console.log(strip` - Disabling ${chalk.bold('application-template-wrapper')}... + if (shouldRunCodemod === undefined) { + console.log(strip` + Disabling ${chalk.bold('application-template-wrapper')}... - This will remove the \`
\` wrapper for the top-level application template (\`${templatePath}\`). + This will remove the \`
\` wrapper for the top-level application template (\`${templatePath}\`). - While this may be a desirable change, it might break your styling in subtle ways: + While this may be a desirable change, it might break your styling in subtle ways: - - Perhaps you were targeting the \`.ember-view\` class in your CSS. + - Perhaps you were targeting the \`.ember-view\` class in your CSS. - - Perhaps you were targeting the \`
\` element in your CSS (e.g. \`body > div > .some-child\`). + - Perhaps you were targeting the \`
\` element in your CSS (e.g. \`body > div > .some-child\`). - - Depending on your choice of \`rootElement\`, your app might not be wrapped inside a block-level element anymore. + - Depending on your choice of \`rootElement\`, your app might not be wrapped inside a block-level element anymore. - For more information, see ${chalk.underline('https://github.com/emberjs/rfcs/pull/280')}. + For more information, see ${chalk.underline('https://github.com/emberjs/rfcs/pull/280')}. - To be very conservative, I could add the \`
\` wrapper to your application.hbs. (You can always remove it later.) - `); + To be very conservative, I could add the \`
\` wrapper to your application.hbs. (You can always remove it later.) + `); - let response = await inquirer.prompt({ - type: 'confirm', - name: 'shouldRewrite', - message: 'Would you like me to do that for you?', - default: false - }); + let response = await inquirer.prompt({ + type: 'confirm', + name: 'shouldRewrite', + message: 'Would you like me to do that for you?', + default: false + }); + + console.log(); - console.log(); + shouldRunCodemod = response.shouldRewrite; + } - if (response.shouldRewrite) { + if (shouldRunCodemod) { let lines = originalContent.split('\n'); let hasFinalNewLine; diff --git a/features/template-only-glimmer-components.js b/features/template-only-glimmer-components.js index ba78186..561923d 100644 --- a/features/template-only-glimmer-components.js +++ b/features/template-only-glimmer-components.js @@ -17,7 +17,7 @@ const ComponentFile = strip` }); `; -/* This forces strip`` to start counting the indentaiton */ +/* This forces strip`` to start counting the indentaton */ const INDENT_START = ''; module.exports = { @@ -25,7 +25,7 @@ module.exports = { url: 'https://github.com/emberjs/rfcs/pull/278', default: false, since: '3.1.0', - callback: async function (project, value) { + callback: async function (project, value, shouldRunCodemod) { if (value !== true) { return; } @@ -108,45 +108,49 @@ module.exports = { return; } - console.log(strip` - Enabling ${chalk.bold('template-only-glimmer-components')}... + if (shouldRunCodemod === undefined) { + console.log(strip` + Enabling ${chalk.bold('template-only-glimmer-components')}... - This will change the semantics for template-only components (components without a \`.js\` file). + This will change the semantics for template-only components (components without a \`.js\` file). - Some notable differences include... + Some notable differences include... - - They will not have a component instance, so statements like \`{{this}}\`, \`{{this.foo}}\` and \`{{foo}}\` will be \`null\` or \`undefined\`. + - They will not have a component instance, so statements like \`{{this}}\`, \`{{this.foo}}\` and \`{{foo}}\` will be \`null\` or \`undefined\`. - - They will not have a wrapper element: what you have in the template will be what is rendered on the screen. + - They will not have a wrapper element: what you have in the template will be what is rendered on the screen. - - Passing classes in the invocation (i.e. \`{{my-component class="..."}}\`) will not work, since there is no wrapper element to apply the classes to. + - Passing classes in the invocation (i.e. \`{{my-component class="..."}}\`) will not work, since there is no wrapper element to apply the classes to. - For more information, see ${chalk.underline('https://github.com/emberjs/rfcs/pull/278')}. + For more information, see ${chalk.underline('https://github.com/emberjs/rfcs/pull/278')}. - While these changes may be desirable for ${chalk.italic('new components')}, they may unexpectedly break the styling or runtime behavior of your ${chalk.italic('existing components')}. + While these changes may be desirable for ${chalk.italic('new components')}, they may unexpectedly break the styling or runtime behavior of your ${chalk.italic('existing components')}. - To be conservative, it is recommended that you add a \`.js\` file for existing template-only components. (You can always delete them later if you aren't relying on the differences.) + To be conservative, it is recommended that you add a \`.js\` file for existing template-only components. (You can always delete them later if you aren't relying on the differences.) - The following components are affected:`); + The following components are affected:`); - for (let i=0; i { }); QUnit.module('feature:disable application-template-wrapper', () => { + QUnit.test('it rewrites application.hbs without prompt when asked to', async function (assert) { + project.write({ + app: { + templates: { + 'application.hbs': strip` +
    +
  • One
    • +
  • Two
    • +
  • Three
    • +
+ + {{outlet}} + + + ` + } + } + }); + + await run('feature:disable', 'application-template-wrapper', '--run-codemod'); + + assert.deepEqual(project.read('app/templates'), { + 'application.hbs': strip` +
+
    +
  • One
    • +
  • Two
    • +
  • Three
    • +
+ + {{outlet}} + + +
+ ` + }, 'it should have rewritten the template with the wrapper'); + }); + QUnit.test('it rewrites application.hbs when asked to', async function (assert) { project.write({ app: { @@ -495,6 +532,14 @@ QUnit.module('commands', hooks => { assert.deepEqual(project.read('app'), CLASSIC_AFTER, 'it should have generated the component JS files'); }); + QUnit.test('it generates component files without prompt when asked to', async function (assert) { + project.write({ app: CLASSIC_BEFORE }); + + await run('feature:enable', 'template-only-glimmer-components', '--run-codemod'); + + assert.deepEqual(project.read('app'), CLASSIC_AFTER, 'it should have generated the component JS files'); + }); + QUnit.test('it works for pods', async function (assert) { project.write({ app: PODS_BEFORE,