From 6d33fe65fe984eaaaa57487f74cbf30e542bb9bf Mon Sep 17 00:00:00 2001 From: Jonathan Johnson Date: Thu, 12 Mar 2020 22:53:33 -0700 Subject: [PATCH 1/2] Add support for data-test-* properties When these are set to boolean true they are added to the element, boolean false are skipped (can't think of why these would exist) When they are set to a literal value that value is used. Anything else throws. --- lib/__tests__/__snapshots__/transform.js.snap | 51 +++++++++++++++++++ lib/__tests__/transform.js | 41 +++++++++++++++ lib/transform/classic.js | 7 +++ lib/transform/native.js | 13 ++++- lib/transform/template.js | 13 ++++- lib/utils/classic.js | 43 ++++++++++++++++ 6 files changed, 165 insertions(+), 3 deletions(-) diff --git a/lib/__tests__/__snapshots__/transform.js.snap b/lib/__tests__/__snapshots__/transform.js.snap index b5b0195..78bdab8 100644 --- a/lib/__tests__/__snapshots__/transform.js.snap +++ b/lib/__tests__/__snapshots__/transform.js.snap @@ -141,6 +141,57 @@ foo ==========" `; +exports[`classic components handles \`data-test-*\` boolean correctly 1`] = ` +"========== + + export default Component.extend({ + 'data-test-one': true, + 'data-test-two': true, + 'data-test-three': false, + }); + +~~~~~~~~~~ +foo +~~~~~~~~~~ + => tagName: div +~~~~~~~~~~ + + export default Component.extend({ + tagName: \\"\\" + }); + +~~~~~~~~~~ +
+ foo +
+==========" +`; + +exports[`classic components handles \`data-test-*\` literals correctly 1`] = ` +"========== + + export default Component.extend({ + 'data-test-item': 1, + 'data-test-name': 'Zoey', + }); + +~~~~~~~~~~ +foo +~~~~~~~~~~ + => tagName: div +~~~~~~~~~~ + + export default Component.extend({ + tagName: \\"\\" + }); + +~~~~~~~~~~ +
+ foo +
+==========" +`; + exports[`classic components handles \`elementId\` correctly 1`] = ` "========== diff --git a/lib/__tests__/transform.js b/lib/__tests__/transform.js index d92d0e7..e66d415 100644 --- a/lib/__tests__/transform.js +++ b/lib/__tests__/transform.js @@ -122,6 +122,47 @@ describe('classic components', function() { expect(generateSnapshot(source, template)).toMatchSnapshot(); }); + test('handles `data-test-*` boolean correctly', () => { + let source = ` + export default Component.extend({ + 'data-test-one': true, + 'data-test-two': true, + 'data-test-three': false, + }); + `; + + let template = `foo`; + + expect(generateSnapshot(source, template)).toMatchSnapshot(); + }); + + test('handles `data-test-*` literals correctly', () => { + let source = ` + export default Component.extend({ + 'data-test-item': 1, + 'data-test-name': 'Zoey', + }); + `; + + let template = `foo`; + + expect(generateSnapshot(source, template)).toMatchSnapshot(); + }); + + test('throws if data-test-* is a computed value', () => { + let source = ` + export default Component.extend({ + 'data-test-computed': computed(function() { + return true; + }), + }); + `; + + expect(() => transform(source, '')).toThrowErrorMatchingInlineSnapshot( + `"Unsupported value for 'data-test-computed'"` + ); + }); + test('throws if `Component.extend({ ... })` argument is not found', () => { let source = ` export default Component.extend(); diff --git a/lib/transform/classic.js b/lib/transform/classic.js index aea07e0..eb2745f 100644 --- a/lib/transform/classic.js +++ b/lib/transform/classic.js @@ -11,6 +11,8 @@ const { findAriaRole, isMethod, isProperty, + isStartsWithProperty, + findDataTestAttributes, } = require('../utils/classic'); const EVENT_HANDLER_METHODS = [ @@ -125,6 +127,9 @@ module.exports = function transformClassicComponent(root, options) { let classNameBindings = findClassNameBindings(properties); debug('classNameBindings: %o', classNameBindings); + let dataTestAttributes = findDataTestAttributes(properties); + debug('dataTestAttributes: %o', dataTestAttributes); + let ariaRole; try { ariaRole = findAriaRole(properties); @@ -155,6 +160,7 @@ module.exports = function transformClassicComponent(root, options) { isProperty(path, 'attributeBindings') || isProperty(path, 'classNames') || isProperty(path, 'classNameBindings') || + isStartsWithProperty(path, 'data-test-') || isProperty(path, 'ariaRole') ) .remove(); @@ -168,6 +174,7 @@ module.exports = function transformClassicComponent(root, options) { classNames, classNameBindings, attributeBindings, + dataTestAttributes, ariaRole, }; }; diff --git a/lib/transform/native.js b/lib/transform/native.js index 566dbf1..7b442ac 100644 --- a/lib/transform/native.js +++ b/lib/transform/native.js @@ -141,6 +141,15 @@ module.exports = function transformNativeComponent(root, options) { .forEach(path => removeDecorator(root, path, 'className', '@ember-decorators/component')); let newSource = root.toSource(); - - return { newSource, tagName, elementId, classNames, classNameBindings, attributeBindings }; + let dataTestAttributes = new Map(); + + return { + newSource, + tagName, + elementId, + classNames, + classNameBindings, + attributeBindings, + dataTestAttributes, + }; }; diff --git a/lib/transform/template.js b/lib/transform/template.js index dbc4706..8bf05de 100644 --- a/lib/transform/template.js +++ b/lib/transform/template.js @@ -8,7 +8,15 @@ const PLACEHOLDER = '@@@PLACEHOLDER@@@'; module.exports = function transformTemplate( template, - { tagName, elementId, classNames, classNameBindings, attributeBindings, ariaRole }, + { + tagName, + elementId, + classNames, + classNameBindings, + attributeBindings, + ariaRole, + dataTestAttributes, + }, options ) { // wrap existing template with root element @@ -50,6 +58,9 @@ module.exports = function transformTemplate( attrs.push(b.attr('class', b.concat(parts))); } + dataTestAttributes.forEach((value, key) => { + attrs.push(b.attr(key, b.text(value))); + }); attrs.push(b.attr('...attributes', b.text(''))); let templateAST = templateRecast.parse(template); diff --git a/lib/utils/classic.js b/lib/utils/classic.js index dd55803..a1de16d 100644 --- a/lib/utils/classic.js +++ b/lib/utils/classic.js @@ -7,6 +7,18 @@ function isProperty(path, name) { return node.type === 'ObjectProperty' && node.key.type === 'Identifier' && node.key.name === name; } +function isStartsWithProperty(path, name) { + let node = path.value; + if (node.type === 'ObjectProperty') { + if (node.key.type === 'Identifier') { + return node.key.name.startsWith(name); + } + if (node.key.type === 'StringLiteral') { + return node.key.value.startsWith(name); + } + } +} + function isMethod(path, name) { let node = path.value; return node.type === 'ObjectMethod' && node.key.type === 'Identifier' && node.key.name === name; @@ -91,8 +103,38 @@ function findClassNameBindings(properties) { return classNameBindings; } +function findDataTestAttributes(properties) { + let nodes = properties.filter(path => isStartsWithProperty(path, 'data-test-')); + if (!nodes) { + return []; + } + const mappedProperties = nodes.map(node => { + return { + name: node.value.key.value, + value: node.value.value.value, + valueType: node.value.value.type, + }; + }); + + let dataTestAttributes = new Map(); + for (let { name, value, valueType } of mappedProperties) { + if (valueType === 'BooleanLiteral') { + if (value) { + dataTestAttributes.set(name, null); + } + } else if (valueType === 'NumericLiteral' || valueType === 'StringLiteral') { + dataTestAttributes.set(name, String(value)); + } else { + throw new SilentError(`Unsupported value for '${name}'`); + } + } + + return dataTestAttributes; +} + module.exports = { isProperty, + isStartsWithProperty, isMethod, findStringProperty, findStringArrayProperties, @@ -100,6 +142,7 @@ module.exports = { findAttributeBindings, findClassNames, findClassNameBindings, + findDataTestAttributes, findElementId, findTagName, }; From 8c06b55458c04e20eaf8d4fd6ecd3c1646f27a8d Mon Sep 17 00:00:00 2001 From: Jonathan Johnson Date: Mon, 30 Mar 2020 20:33:23 -0700 Subject: [PATCH 2/2] Only run data-test transform if the ember-test-selectors addon is present --- lib/__tests__/__snapshots__/transform.js.snap | 27 +++++++++++++++++++ lib/__tests__/transform.js | 23 ++++++++++++---- lib/index.js | 5 +++- lib/transform/classic.js | 2 +- lib/transform/template.js | 8 +++--- 5 files changed, 55 insertions(+), 10 deletions(-) diff --git a/lib/__tests__/__snapshots__/transform.js.snap b/lib/__tests__/__snapshots__/transform.js.snap index 78bdab8..49b63a4 100644 --- a/lib/__tests__/__snapshots__/transform.js.snap +++ b/lib/__tests__/__snapshots__/transform.js.snap @@ -240,6 +240,33 @@ foo ==========" `; +exports[`classic components handles \`hasTestSelectors\` option correctly 1`] = ` +"========== + + export default Component.extend({ + 'data-test-item': 1, + 'data-test-name': 'Zoey', + }); + +~~~~~~~~~~ +foo +~~~~~~~~~~ + => tagName: div +~~~~~~~~~~ + + export default Component.extend({ + tagName: \\"\\", + 'data-test-item': 1, + 'data-test-name': 'Zoey' + }); + +~~~~~~~~~~ +
+ foo +
+==========" +`; + exports[`classic components handles single \`classNames\` item correctly 1`] = ` "========== diff --git a/lib/__tests__/transform.js b/lib/__tests__/transform.js index e66d415..debab96 100644 --- a/lib/__tests__/transform.js +++ b/lib/__tests__/transform.js @@ -133,7 +133,7 @@ describe('classic components', function() { let template = `foo`; - expect(generateSnapshot(source, template)).toMatchSnapshot(); + expect(generateSnapshot(source, template, { hasTestSelectors: true })).toMatchSnapshot(); }); test('handles `data-test-*` literals correctly', () => { @@ -146,7 +146,7 @@ describe('classic components', function() { let template = `foo`; - expect(generateSnapshot(source, template)).toMatchSnapshot(); + expect(generateSnapshot(source, template, { hasTestSelectors: true })).toMatchSnapshot(); }); test('throws if data-test-* is a computed value', () => { @@ -158,9 +158,22 @@ describe('classic components', function() { }); `; - expect(() => transform(source, '')).toThrowErrorMatchingInlineSnapshot( - `"Unsupported value for 'data-test-computed'"` - ); + expect(() => + transform(source, '', { hasTestSelectors: true }) + ).toThrowErrorMatchingInlineSnapshot(`"Unsupported value for 'data-test-computed'"`); + }); + + test('handles `hasTestSelectors` option correctly', () => { + let source = ` + export default Component.extend({ + 'data-test-item': 1, + 'data-test-name': 'Zoey', + }); + `; + + let template = `foo`; + + expect(generateSnapshot(source, template, { hasTestSelectors: false })).toMatchSnapshot(); }); test('throws if `Component.extend({ ... })` argument is not found', () => { diff --git a/lib/index.js b/lib/index.js index 4fff6e8..c5f5ade 100644 --- a/lib/index.js +++ b/lib/index.js @@ -44,10 +44,13 @@ async function runForGlobs(patterns, cwd, options = {}) { let hasComponentCSS = (pkg.dependencies && pkg.dependencies['ember-component-css']) || (pkg.devDependencies && pkg.devDependencies['ember-component-css']); + let hasTestSelectors = + (pkg.dependencies && pkg.dependencies['ember-test-selectors']) || + (pkg.devDependencies && pkg.devDependencies['ember-test-selectors']); for (let path of paths) { try { - let tagName = transformPath(path, { hasComponentCSS }); + let tagName = transformPath(path, { hasComponentCSS, hasTestSelectors }); if (tagName) { log(chalk.green(`${chalk.dim(path)}: <${tagName}>...`)); } else { diff --git a/lib/transform/classic.js b/lib/transform/classic.js index eb2745f..0fd13b3 100644 --- a/lib/transform/classic.js +++ b/lib/transform/classic.js @@ -160,7 +160,7 @@ module.exports = function transformClassicComponent(root, options) { isProperty(path, 'attributeBindings') || isProperty(path, 'classNames') || isProperty(path, 'classNameBindings') || - isStartsWithProperty(path, 'data-test-') || + (options.hasTestSelectors && isStartsWithProperty(path, 'data-test-')) || isProperty(path, 'ariaRole') ) .remove(); diff --git a/lib/transform/template.js b/lib/transform/template.js index 8bf05de..47a0b85 100644 --- a/lib/transform/template.js +++ b/lib/transform/template.js @@ -58,9 +58,11 @@ module.exports = function transformTemplate( attrs.push(b.attr('class', b.concat(parts))); } - dataTestAttributes.forEach((value, key) => { - attrs.push(b.attr(key, b.text(value))); - }); + if (options.hasTestSelectors) { + dataTestAttributes.forEach((value, key) => { + attrs.push(b.attr(key, b.text(value))); + }); + } attrs.push(b.attr('...attributes', b.text(''))); let templateAST = templateRecast.parse(template);