Skip to content

Commit

Permalink
Add --run-codemod commandline option.
Browse files Browse the repository at this point in the history
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).
  • Loading branch information
rwjblue committed Dec 19, 2019
1 parent b34d1f6 commit 419b9a9
Show file tree
Hide file tree
Showing 4 changed files with 123 additions and 52 deletions.
30 changes: 24 additions & 6 deletions commands/index.js
Original file line number Diff line number Diff line change
Expand Up @@ -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) {
Expand All @@ -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 = {};
Expand Down Expand Up @@ -134,23 +134,41 @@ 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: [
'<feature-name>'
],
run(_, args) {
return this._setFeature(args[0], true);
run(commandOptions, args) {
return this._setFeature(args[0], true, commandOptions.runCodemod);
}
}, SHARED);

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: [
'<feature-name>'
],
run(_, args) {
return this._setFeature(args[0], false);
run(commandOptions, args) {
return this._setFeature(args[0], false, commandOptions.runCodemod);
}
}, SHARED);

Expand Down
42 changes: 23 additions & 19 deletions features/application-template-wrapper.js
Original file line number Diff line number Diff line change
Expand Up @@ -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;
}
Expand Down Expand Up @@ -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 \`<div class="ember-view">\` wrapper for the top-level application template (\`${templatePath}\`).
This will remove the \`<div class="ember-view">\` 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 \`<div>\` element in your CSS (e.g. \`body > div > .some-child\`).
- Perhaps you were targeting the \`<div>\` 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 \`<div class="ember-view">\` wrapper to your application.hbs. (You can always remove it later.)
`);
To be very conservative, I could add the \`<div class="ember-view">\` 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;

Expand Down
54 changes: 29 additions & 25 deletions features/template-only-glimmer-components.js
Original file line number Diff line number Diff line change
Expand Up @@ -17,15 +17,15 @@ const ComponentFile = strip`
});
`;

/* This forces strip`` to start counting the indentaiton */
/* This forces strip`` to start counting the indentaton */
const INDENT_START = '';

module.exports = {
description: 'Use Glimmer Components semantics for template-only components (component templates with no corresponding .js file).',
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;
}
Expand Down Expand Up @@ -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<templates.length; i++) {
console.log(strip`
for (let i=0; i<templates.length; i++) {
console.log(strip`
${INDENT_START}
- ${chalk.underline(templates[i])}
${chalk.gray(`(Recommendation: add ${chalk.cyan.underline(components[i])})`)}
`);
}
`);
}

let response = await inquirer.prompt({
type: 'confirm',
name: 'shouldGenerate',
message: 'Would you like me to generate these component files for you?',
default: true
});
let response = await inquirer.prompt({
type: 'confirm',
name: 'shouldGenerate',
message: 'Would you like me to generate these component files for you?',
default: true
});

shouldRunCodemod = response.shouldGenerate;

console.log();
console.log();
}

if (response.shouldGenerate) {
if (shouldRunCodemod) {
for (let i=0; i<components.length; i++) {
let componentPath = components[i];
console.log(` ${chalk.green('create')} ${componentPath}`);
Expand Down
49 changes: 47 additions & 2 deletions tests/commands-test.js
Original file line number Diff line number Diff line change
Expand Up @@ -11,8 +11,7 @@ const strip = require('../utils').strip;

const FEATURES = require('../features');

function run(/*command, ...args, options */) {
let args = [].slice.call(arguments);
function run(...args) {
let options = {};

if (typeof args[args.length - 1] === 'object') {
Expand Down Expand Up @@ -241,6 +240,44 @@ QUnit.module('commands', hooks => {
});

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`
<ul>
<li>One</li>
<li>Two</li>
<li>Three</li>
</ul>
{{outlet}}
<!-- wow -->
`
}
}
});

await run('feature:disable', 'application-template-wrapper', '--run-codemod');

assert.deepEqual(project.read('app/templates'), {
'application.hbs': strip`
<div class="ember-view">
<ul>
<li>One</li>
<li>Two</li>
<li>Three</li>
</ul>
{{outlet}}
<!-- wow -->
</div>
`
}, 'it should have rewritten the template with the wrapper');
});

QUnit.test('it rewrites application.hbs when asked to', async function (assert) {
project.write({
app: {
Expand Down Expand Up @@ -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,
Expand Down

0 comments on commit 419b9a9

Please sign in to comment.