From 702dd40ad2c030c01ed001e648b556c7c7c46495 Mon Sep 17 00:00:00 2001 From: Suchita Doshi Date: Sun, 23 Feb 2020 21:14:24 -0800 Subject: [PATCH 1/6] Add support for prefixing `this` only for owned properties --- bin/cli.js | 93 +++++++++++++- package.json | 1 + .../__testfixtures__/-mock-telemetry.json | 25 ++-- ...prefix-component-properties-only.input.hbs | 10 ++ ...fix-component-properties-only.options.json | 3 + ...refix-component-properties-only.output.hbs | 10 ++ transforms/no-implicit-this/helpers/plugin.js | 62 +++++----- transforms/no-implicit-this/helpers/util.js | 115 ++++++++++++++++++ transforms/no-implicit-this/index.js | 3 +- transforms/no-implicit-this/test-helpers.js | 12 +- yarn.lock | 5 + 11 files changed, 282 insertions(+), 57 deletions(-) create mode 100644 transforms/no-implicit-this/__testfixtures__/handlebars-with-prefix-component-properties-only.input.hbs create mode 100644 transforms/no-implicit-this/__testfixtures__/handlebars-with-prefix-component-properties-only.options.json create mode 100644 transforms/no-implicit-this/__testfixtures__/handlebars-with-prefix-component-properties-only.output.hbs create mode 100644 transforms/no-implicit-this/helpers/util.js diff --git a/bin/cli.js b/bin/cli.js index e0ab4b39..89272a7f 100755 --- a/bin/cli.js +++ b/bin/cli.js @@ -2,19 +2,100 @@ 'use strict'; const debug = require('debug')('ember-no-implicit-this-codemod'); -const { - gatherTelemetryForUrl, - analyzeEmberObject, - getTelemetry, -} = require('ember-codemods-telemetry-helpers'); +const globby = require('globby'); +const finder = require('find-package-json'); +const recast = require('ember-template-recast'); +const fs = require('fs'); +const path = require('path'); +const { appResolver, detectTypeAndName } = require('../transforms/no-implicit-this/helpers/util'); +const { gatherSingleTelemetryForUrl, getTelemetry } = require('ember-codemods-telemetry-helpers'); const appLocation = process.argv[2]; const args = process.argv.slice(3); +/** + * Pre parse the template to collect information for potential helper/components + * We are pushing the lookup names for any `PathExpression` occuring in: + * - MustacheStatement: It could be helper or component + * - BlockStatement: It can only be a component + * - SubExpression: It can only be a helper + * The values from this lookup array will be consumed by the app's resolver. + * If the lookup name is is found in the app's registry, it would help + * determine local properties for a given template's backing js class. + * @param {*} root + * @param {*} lookupNames + */ +function _preparseTemplate(root) { + let lookupNames = []; + recast.traverse(root, { + MustacheStatement(node) { + if (node.path.type === 'PathExpression') { + lookupNames.push({ lookupName: `component:${node.path.original}` }); + lookupNames.push({ lookupName: `helper:${node.path.original}` }); + } + }, + + BlockStatement(node) { + if (node.path.type === 'PathExpression') { + lookupNames.push({ lookupName: `component:${node.path.original}` }); + } + }, + + SubExpression(node) { + if (node.path.type === 'PathExpression') { + lookupNames.push({ lookupName: `helper:${node.path.original}` }); + } + }, + }); + return lookupNames; +} + +/** + * Return the app name based on the package json, keep calling the function + * recursively until you reach the root of the app. + * @param {*} f + */ +function findAppName(f) { + let fileName = f.next().value; + if (fileName.keywords && fileName.keywords.includes('ember-addon')) { + return findAppName(f); + } else if (Object.keys(fileName.devDependencies).includes('ember-cli')) { + // There could be cases where the root package.json might have multiple Ember apps within. + return fileName['ember-addon'].apps ? fileName['ember-addon'].apps : [fileName.name]; + } +} + (async () => { + const filePaths = globby.sync(args[0], { ignore: 'node_modules/**' }); + + // Get the package.json for the first file path and pass it to the `findAppName` function. + // Note: We just need the first found file path since from there we would be able + // to get the root level app name. + const appName = filePaths ? findAppName(finder(filePaths[0])) : null; + + let lookupNames = filePaths.map(detectTypeAndName).filter(item => item !== null); + // Pre-parse the each template file. + for (let i = 0; i < filePaths.length; i++) { + let filePath = filePaths[i]; + let extension = path.extname(filePath); + + if (!['.hbs'].includes(extension.toLowerCase())) { + // do nothing on non-hbs files + continue; + } + + let code = fs.readFileSync(filePath).toString(); + let root = recast.parse(code); + + lookupNames = lookupNames.concat(_preparseTemplate(root, lookupNames, filePath)); + } + debug('Gathering telemetry data from %s ...', appLocation); - await gatherTelemetryForUrl(appLocation, analyzeEmberObject); + + // This is for collecting metadata for the app just once to generate the map of lookupnames to local properties + await gatherSingleTelemetryForUrl(appLocation, appResolver, lookupNames, appName); let telemetry = getTelemetry(); + debug('Gathered telemetry on %d modules', Object.keys(telemetry).length); require('codemod-cli').runTransform(__dirname, 'no-implicit-this', args, 'hbs'); diff --git a/package.json b/package.json index 9ea0fe06..446bc2f3 100644 --- a/package.json +++ b/package.json @@ -49,6 +49,7 @@ "eslint-config-prettier": "^6.9.0", "eslint-plugin-prettier": "^3.1.2", "execa": "^3.4.0", + "find-package-json": "^1.2.0", "jest": "^24.9.0", "prettier": "^1.19.1", "release-it": "^12.4.2", diff --git a/transforms/no-implicit-this/__testfixtures__/-mock-telemetry.json b/transforms/no-implicit-this/__testfixtures__/-mock-telemetry.json index b04d6985..03bd3e3b 100644 --- a/transforms/no-implicit-this/__testfixtures__/-mock-telemetry.json +++ b/transforms/no-implicit-this/__testfixtures__/-mock-telemetry.json @@ -1,11 +1,18 @@ { - "some-component": { "type": "Component" }, - "my-component": { "type": "Component" }, - "namespace/my-component": { "type": "Component" }, - "block-component": { "type": "Component" }, - "foo": { "type": "Component" }, - "namespace/foo": { "type": "Component" }, - "my-helper": { "type": "Helper" }, - "a-helper": { "type": "Helper" }, - "foo-bar-baz": { "type": "Component" } + "single-telemetry": { + "component:handlebars-with-prefix-component-properties-only": { + "type": "Component", + "localProperties": ["baz", "bang"], + "filePath": "transforms/no-implicit-this/__testfixtures__/handlebars-with-prefix-component-properties-only.hbs" + }, + "some-component": { "type": "Component" }, + "my-component": { "type": "Component" }, + "namespace/my-component": { "type": "Component" }, + "block-component": { "type": "Component" }, + "foo": { "type": "Component" }, + "namespace/foo": { "type": "Component" }, + "my-helper": { "type": "Helper" }, + "a-helper": { "type": "Helper" }, + "foo-bar-baz": { "type": "Component" } + } } diff --git a/transforms/no-implicit-this/__testfixtures__/handlebars-with-prefix-component-properties-only.input.hbs b/transforms/no-implicit-this/__testfixtures__/handlebars-with-prefix-component-properties-only.input.hbs new file mode 100644 index 00000000..761d6f92 --- /dev/null +++ b/transforms/no-implicit-this/__testfixtures__/handlebars-with-prefix-component-properties-only.input.hbs @@ -0,0 +1,10 @@ +{{my-component "string"}} +{{my-component 1}} +{{my-component foo}} +{{my-component baz}} +{{my-component bang}} +{{my-component @foo}} +{{my-component property}} +{{my-component (my-helper baz)}} +{{my-component (my-helper 1)}} +{{get this 'key'}} \ No newline at end of file diff --git a/transforms/no-implicit-this/__testfixtures__/handlebars-with-prefix-component-properties-only.options.json b/transforms/no-implicit-this/__testfixtures__/handlebars-with-prefix-component-properties-only.options.json new file mode 100644 index 00000000..18dcf8f4 --- /dev/null +++ b/transforms/no-implicit-this/__testfixtures__/handlebars-with-prefix-component-properties-only.options.json @@ -0,0 +1,3 @@ +{ + "noStrict": "true" +} diff --git a/transforms/no-implicit-this/__testfixtures__/handlebars-with-prefix-component-properties-only.output.hbs b/transforms/no-implicit-this/__testfixtures__/handlebars-with-prefix-component-properties-only.output.hbs new file mode 100644 index 00000000..294f302c --- /dev/null +++ b/transforms/no-implicit-this/__testfixtures__/handlebars-with-prefix-component-properties-only.output.hbs @@ -0,0 +1,10 @@ +{{my-component "string"}} +{{my-component 1}} +{{my-component foo}} +{{my-component this.baz}} +{{my-component this.bang}} +{{my-component @foo}} +{{my-component property}} +{{my-component (my-helper this.baz)}} +{{my-component (my-helper 1)}} +{{get this 'key'}} \ No newline at end of file diff --git a/transforms/no-implicit-this/helpers/plugin.js b/transforms/no-implicit-this/helpers/plugin.js index 864c6841..5d3ae6e1 100644 --- a/transforms/no-implicit-this/helpers/plugin.js +++ b/transforms/no-implicit-this/helpers/plugin.js @@ -1,11 +1,16 @@ const debug = require('debug')('ember-no-implicit-this-codemod:plugin'); const recast = require('ember-template-recast'); +const path = require('path'); // everything is copy-pasteable to astexplorer.net. // sorta. telemetry needs to be defined. // telemtry can be populated with -mock-telemetry.json const KNOWN_HELPERS = require('./known-helpers'); +function getTelemetryObjByName(name, telemetry) { + let telemetryLookupName = Object.keys(telemetry).find(item => item.split(':').pop() === name); + return telemetry[telemetryLookupName] || {}; +} /** * plugin entrypoint */ @@ -13,8 +18,7 @@ function transform(root, options = {}) { let b = recast.builders; let scopedParams = []; - let telemetry = options.telemetry || {}; - let [components, helpers] = populateInvokeables(telemetry); + let telemetry = options.telemetry ? options.telemetry['single-telemetry'] : {}; let customHelpers = options.customHelpers || []; @@ -67,6 +71,24 @@ function transform(root, options = {}) { return; } + // check for the flag for stricter prefixing. This check ensures that it only + // prefixes `this` to the properties owned by the backing JS class of the template. + if (options.noStrict === 'true') { + const matchedFilePath = Object.keys(telemetry).find( + item => telemetry[item].filePath === path.relative(process.cwd(), options.filePath) + ); + + if (matchedFilePath) { + let lookupObject = telemetry[matchedFilePath]; + const ownProperties = lookupObject ? lookupObject.localProperties : []; + + if (!ownProperties.includes(firstPart)) { + debug(`Skipping \`%s\` because it is not a local property`, node.original); + return; + } + } + } + // skip `hasBlock` keyword if (node.original === 'hasBlock') { debug(`Skipping \`%s\` because it is a keyword`, node.original); @@ -89,10 +111,10 @@ function transform(root, options = {}) { return true; } - let helper = helpers.find(path => path.endsWith(`/${name}`)); - if (helper) { - let message = `Skipping \`%s\` because it appears to be a helper from the telemetry data: %s`; - debug(message, name, helper); + const telemetryObj = getTelemetryObjByName(name, telemetry); + if (telemetryObj.type === 'Helper') { + let message = `Skipping \`%s\` because it appears to be a helper from the lookup object`; + debug(message, name); return true; } @@ -100,10 +122,10 @@ function transform(root, options = {}) { } function isComponent(name) { - let component = components.find(path => path.endsWith(`/${name}`)); - if (component) { - let message = `Skipping \`%s\` because it appears to be a component from the telemetry data: %s`; - debug(message, name, component); + const telemetryObj = getTelemetryObjByName(name, telemetry); + if (telemetryObj.type === 'Component') { + let message = `Skipping \`%s\` because it appears to be a component from the lookup object`; + debug(message, name); return true; } @@ -189,24 +211,4 @@ function transform(root, options = {}) { }); } -function populateInvokeables(telemetry) { - let components = []; - let helpers = []; - - for (let name of Object.keys(telemetry)) { - let entry = telemetry[name]; - - switch (entry.type) { - case 'Component': - components.push(name); - break; - case 'Helper': - helpers.push(name); - break; - } - } - - return [components, helpers]; -} - module.exports = transform; diff --git a/transforms/no-implicit-this/helpers/util.js b/transforms/no-implicit-this/helpers/util.js new file mode 100644 index 00000000..eda4d2e7 --- /dev/null +++ b/transforms/no-implicit-this/helpers/util.js @@ -0,0 +1,115 @@ +const path = require('path'); +const globby = require('globby'); + +/** + * Generates a lookup name for a backing js class. + * @param {*} matchedItem + * @param {*} fileName + * @param {*} type + */ +function getDetectedName(matchedItem, fileName, type) { + let detectedName = null; + // create regex string to derive the potential addon name and generated the + // lookup name. + let regexString = `(.*)/(addon|app)/${type}s(.*)/${fileName}.js`; + let matchedRegex = matchedItem.match(regexString); + if (matchedRegex) { + const folderType = matchedRegex[2]; + const rootName = matchedRegex[1].split('/').pop(); + detectedName = + folderType === 'addon' ? `${rootName}@${type}:${fileName}` : `${type}:${fileName}`; + } + return detectedName; +} + +/** + * Returns lookup name for a given file path (template file) by searching for a backing + * js file for the template. + * @param {*} entry + */ +function detectTypeAndName(entry) { + let detectedComponentName = null; + let detectedHelperName = null; + let detectedControllerName = null; + const fileName = path.basename(entry).split('.')[0]; + // Since we care about the component and helpers (as we do not want to prefix `this`) and + // also we would need to generate lookupNames for controllers and components pertaining to the + // current template file, do a globby search and gather filepaths that match these folders. + const matched = globby.sync( + [ + `**/components/**/${fileName}.js`, + `**/helpers/**/${fileName}.js`, + `**/controllers/**/${fileName}.js`, + ], + { + ignore: ['node_modules/**'], + } + ); + if (matched.length) { + matched.forEach(matchedItem => { + detectedComponentName = getDetectedName(matchedItem, fileName, 'component'); + detectedHelperName = getDetectedName(matchedItem, fileName, 'helper'); + detectedControllerName = getDetectedName(matchedItem, fileName, 'controller'); + }); + } + let detectedName = detectedComponentName || detectedHelperName || detectedControllerName; + return { lookupName: detectedName, filePath: entry }; +} + +/** + * Analyzes the app and collects local properties and type of the lookup name. + * Returns the map of lookupName to its metadata. + * { + * "component:foo": { localProperties: ['foo', 'bar'], type: 'Component', filePath: 'app/components/foo.js' } + * } + * @param {*} lookupNames + * @param {*} appname + */ +function appResolver(lookupNames, currAppName) { + let mapping = {}; + if (Array.isArray(currAppName)) { + currAppName.forEach(appItem => { + try { + let app = require(`${appItem}/app`).default.create({ autoboot: false }); + let localProperties = []; + lookupNames.forEach(item => { + // Resolve the class from the lookup name, if found, then check if its a helper, component + // or controller and add the local properties & the type to the map. + let klass = app.resolveRegistration(item.lookupName); + if (klass) { + if (klass.proto) { + const protoInfo = klass.proto(); + localProperties = Object.keys(protoInfo).filter( + key => !['_super', 'actions'].includes(key) + ); + /* globals Ember */ + // Determine the type of the class's instance. + let klassType = null; + if (protoInfo instanceof Ember['Controller']) { + klassType = 'Controller'; + } else if (protoInfo instanceof Ember['Component']) { + klassType = 'Component'; + } + + // Create a map with lookupName as key with meta information. + mapping[item.lookupName] = { + filePath: item.filePath, + localProperties, + type: klassType, + }; + } else if (klass.isHelperFactory) { + mapping[item.lookupName] = { type: 'Helper', filePath: item.filePath }; + } + } + }); + app.destroy(); + } catch (e) { + console.log(e); + } + }); + } + + return mapping; +} + +module.exports = { appResolver, detectTypeAndName }; diff --git a/transforms/no-implicit-this/index.js b/transforms/no-implicit-this/index.js index 1bd55ff1..42a4fd3b 100644 --- a/transforms/no-implicit-this/index.js +++ b/transforms/no-implicit-this/index.js @@ -35,6 +35,7 @@ function _getCustomHelpersFromConfig(configPath) { function getOptions() { let cliOptions = getCLIOptions(); let options = { + noStrict: cliOptions.noStrict, customHelpers: _getCustomHelpersFromConfig(cliOptions.config), telemetry: getTelemetry(), }; @@ -43,7 +44,7 @@ function getOptions() { module.exports = function transformer(file /*, api */) { let extension = path.extname(file.path); - let options = Object.assign({}, DEFAULT_OPTIONS, getOptions()); + let options = Object.assign({}, DEFAULT_OPTIONS, getOptions(), { filePath: file.path }); if (!['.hbs'].includes(extension.toLowerCase())) { debug('Skipping %s because it does not match the .hbs file extension', file.path); diff --git a/transforms/no-implicit-this/test-helpers.js b/transforms/no-implicit-this/test-helpers.js index ed118497..b82b6b9c 100644 --- a/transforms/no-implicit-this/test-helpers.js +++ b/transforms/no-implicit-this/test-helpers.js @@ -1,18 +1,8 @@ -const path = require('path'); const { setTelemetry } = require('ember-codemods-telemetry-helpers'); const mockTelemetryData = require('./__testfixtures__/-mock-telemetry.json'); function setupTelemetry() { - let mockTelemetry = {}; - - Object.keys(mockTelemetryData).forEach(key => { - let value = mockTelemetryData[key] || {}; - let mockPath = path.resolve(__dirname, `./__testfixtures__/${key}`); - - mockTelemetry[mockPath] = value; - }); - - setTelemetry(mockTelemetry); + setTelemetry(mockTelemetryData); } module.exports = { setupTelemetry }; diff --git a/yarn.lock b/yarn.lock index 4b6e11ca..d028925c 100644 --- a/yarn.lock +++ b/yarn.lock @@ -2772,6 +2772,11 @@ find-cache-dir@^2.0.0: make-dir "^2.0.0" pkg-dir "^3.0.0" +find-package-json@^1.2.0: + version "1.2.0" + resolved "https://registry.yarnpkg.com/find-package-json/-/find-package-json-1.2.0.tgz#4057d1b943f82d8445fe52dc9cf456f6b8b58083" + integrity sha512-+SOGcLGYDJHtyqHd87ysBhmaeQ95oWspDKnMXBrnQ9Eq4OkLNqejgoaD8xVWu6GPa0B6roa6KinCMEMcVeqONw== + find-up@4.1.0, find-up@^4.0.0: version "4.1.0" resolved "https://registry.yarnpkg.com/find-up/-/find-up-4.1.0.tgz#97afe7d6cdc0bc5928584b7c8d7b16e8a9aa5d19" From e052e285ac7b03db14de2c4f89e73d9a13f46cb6 Mon Sep 17 00:00:00 2001 From: Suchita Doshi Date: Sun, 23 Feb 2020 22:07:42 -0800 Subject: [PATCH 2/6] add additional check --- bin/cli.js | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/bin/cli.js b/bin/cli.js index 89272a7f..c20782bc 100755 --- a/bin/cli.js +++ b/bin/cli.js @@ -59,8 +59,9 @@ function findAppName(f) { if (fileName.keywords && fileName.keywords.includes('ember-addon')) { return findAppName(f); } else if (Object.keys(fileName.devDependencies).includes('ember-cli')) { + let emberAddon = fileName['ember-addon']; // There could be cases where the root package.json might have multiple Ember apps within. - return fileName['ember-addon'].apps ? fileName['ember-addon'].apps : [fileName.name]; + return emberAddon && emberAddon.apps ? emberAddon.apps : [fileName.name]; } } From 084c92221a95b3ebbabbcac368ed7350a1d93ff6 Mon Sep 17 00:00:00 2001 From: Suchita Doshi Date: Thu, 23 Apr 2020 21:54:06 -0700 Subject: [PATCH 3/6] update the code based on telemetry data changes --- bin/cli.js | 14 ++++++++++++-- .../__testfixtures__/-mock-telemetry.json | 2 +- transforms/no-implicit-this/helpers/plugin.js | 3 ++- transforms/no-implicit-this/helpers/util.js | 3 ++- 4 files changed, 17 insertions(+), 5 deletions(-) diff --git a/bin/cli.js b/bin/cli.js index c20782bc..30e8e73c 100755 --- a/bin/cli.js +++ b/bin/cli.js @@ -7,7 +7,11 @@ const finder = require('find-package-json'); const recast = require('ember-template-recast'); const fs = require('fs'); const path = require('path'); -const { appResolver, detectTypeAndName } = require('../transforms/no-implicit-this/helpers/util'); +const { + appResolver, + detectTypeAndName, + TELEMETRY_KEY, +} = require('../transforms/no-implicit-this/helpers/util'); const { gatherSingleTelemetryForUrl, getTelemetry } = require('ember-codemods-telemetry-helpers'); const appLocation = process.argv[2]; const args = process.argv.slice(3); @@ -93,7 +97,13 @@ function findAppName(f) { debug('Gathering telemetry data from %s ...', appLocation); // This is for collecting metadata for the app just once to generate the map of lookupnames to local properties - await gatherSingleTelemetryForUrl(appLocation, appResolver, lookupNames, appName); + await gatherSingleTelemetryForUrl( + appLocation, + { telemetryKey: TELEMETRY_KEY }, + appResolver, + lookupNames, + appName + ); let telemetry = getTelemetry(); diff --git a/transforms/no-implicit-this/__testfixtures__/-mock-telemetry.json b/transforms/no-implicit-this/__testfixtures__/-mock-telemetry.json index 03bd3e3b..6aeb03c0 100644 --- a/transforms/no-implicit-this/__testfixtures__/-mock-telemetry.json +++ b/transforms/no-implicit-this/__testfixtures__/-mock-telemetry.json @@ -1,5 +1,5 @@ { - "single-telemetry": { + "telemetry-data": { "component:handlebars-with-prefix-component-properties-only": { "type": "Component", "localProperties": ["baz", "bang"], diff --git a/transforms/no-implicit-this/helpers/plugin.js b/transforms/no-implicit-this/helpers/plugin.js index 5d3ae6e1..92d45350 100644 --- a/transforms/no-implicit-this/helpers/plugin.js +++ b/transforms/no-implicit-this/helpers/plugin.js @@ -6,6 +6,7 @@ const path = require('path'); // sorta. telemetry needs to be defined. // telemtry can be populated with -mock-telemetry.json const KNOWN_HELPERS = require('./known-helpers'); +const { TELEMETRY_KEY } = require('./util'); function getTelemetryObjByName(name, telemetry) { let telemetryLookupName = Object.keys(telemetry).find(item => item.split(':').pop() === name); @@ -18,7 +19,7 @@ function transform(root, options = {}) { let b = recast.builders; let scopedParams = []; - let telemetry = options.telemetry ? options.telemetry['single-telemetry'] : {}; + let telemetry = options.telemetry ? options.telemetry[TELEMETRY_KEY] : {}; let customHelpers = options.customHelpers || []; diff --git a/transforms/no-implicit-this/helpers/util.js b/transforms/no-implicit-this/helpers/util.js index eda4d2e7..ff93e4ce 100644 --- a/transforms/no-implicit-this/helpers/util.js +++ b/transforms/no-implicit-this/helpers/util.js @@ -1,5 +1,6 @@ const path = require('path'); const globby = require('globby'); +const TELEMETRY_KEY = 'telemetry-data'; /** * Generates a lookup name for a backing js class. @@ -112,4 +113,4 @@ function appResolver(lookupNames, currAppName) { return mapping; } -module.exports = { appResolver, detectTypeAndName }; +module.exports = { appResolver, detectTypeAndName, TELEMETRY_KEY }; From 89526846ca2fd8439f1ae41c6e72ad4056e5fd46 Mon Sep 17 00:00:00 2001 From: Suchita Doshi Date: Fri, 24 Apr 2020 08:30:37 -0700 Subject: [PATCH 4/6] bump telemetry helpers --- package.json | 2 +- yarn.lock | 8 ++++---- 2 files changed, 5 insertions(+), 5 deletions(-) diff --git a/package.json b/package.json index 446bc2f3..6ddf3c4e 100644 --- a/package.json +++ b/package.json @@ -33,7 +33,7 @@ "dependencies": { "codemod-cli": "^2.1.0", "debug": "^4.1.1", - "ember-codemods-telemetry-helpers": "^1.1.0", + "ember-codemods-telemetry-helpers": "^1.2.0", "ember-template-recast": "^3.3.2" }, "devDependencies": { diff --git a/yarn.lock b/yarn.lock index d028925c..1781e57c 100644 --- a/yarn.lock +++ b/yarn.lock @@ -2266,10 +2266,10 @@ electron-to-chromium@^1.3.295: resolved "https://registry.yarnpkg.com/electron-to-chromium/-/electron-to-chromium-1.3.296.tgz#a1d4322d742317945285d3ba88966561b67f3ac8" integrity sha512-s5hv+TSJSVRsxH190De66YHb50pBGTweT9XGWYu/LMR20KX6TsjFzObo36CjVAzM+PUeeKSBRtm/mISlCzeojQ== -ember-codemods-telemetry-helpers@^1.1.0: - version "1.1.0" - resolved "https://registry.yarnpkg.com/ember-codemods-telemetry-helpers/-/ember-codemods-telemetry-helpers-1.1.0.tgz#49249fb18f226a18a6921b12785672b39325a104" - integrity sha512-c5bDeJ/pfGRaQnq58CLHA5Nv318G50P30R9LPjBaWvyamRfJp1lVjhylLEj2VQlt/IGskXUk12Un0DORqKYFzw== +ember-codemods-telemetry-helpers@^1.2.0: + version "1.2.0" + resolved "https://registry.yarnpkg.com/ember-codemods-telemetry-helpers/-/ember-codemods-telemetry-helpers-1.2.0.tgz#d88541ee3f75de3f4094a40ce6a1d09df72dcbb4" + integrity sha512-m7D9pNbGC2EhoZ70xP/Jn5oWcpXFi16J4BKk810bnnGXVxx4Vl2YuKSs+SaAjEFnBIyqU0o4CXeCftBmy4Q7HA== dependencies: fs-extra "^8.1.0" git-repo-info "^2.1.0" From 7eccb5a2e4e2e08ac5cb9fd1f3388ce7415fa574 Mon Sep 17 00:00:00 2001 From: Suchita Doshi Date: Fri, 24 Apr 2020 11:04:07 -0700 Subject: [PATCH 5/6] bump telemetry version and fix tests --- bin/cli.js | 14 ++++----- package.json | 2 +- .../__testfixtures__/-mock-telemetry.json | 30 +++++++++---------- transforms/no-implicit-this/helpers/plugin.js | 3 +- transforms/no-implicit-this/helpers/util.js | 7 +++-- transforms/no-implicit-this/index.js | 3 +- transforms/no-implicit-this/test-helpers.js | 5 ++-- yarn.lock | 8 ++--- 8 files changed, 36 insertions(+), 36 deletions(-) diff --git a/bin/cli.js b/bin/cli.js index 30e8e73c..181bf062 100755 --- a/bin/cli.js +++ b/bin/cli.js @@ -28,25 +28,25 @@ const args = process.argv.slice(3); * @param {*} root * @param {*} lookupNames */ -function _preparseTemplate(root) { +function _preparseTemplate(root, filePath) { let lookupNames = []; recast.traverse(root, { MustacheStatement(node) { if (node.path.type === 'PathExpression') { - lookupNames.push({ lookupName: `component:${node.path.original}` }); - lookupNames.push({ lookupName: `helper:${node.path.original}` }); + lookupNames.push({ lookupName: `component:${node.path.original}`, filePath }); + lookupNames.push({ lookupName: `helper:${node.path.original}`, filePath }); } }, BlockStatement(node) { if (node.path.type === 'PathExpression') { - lookupNames.push({ lookupName: `component:${node.path.original}` }); + lookupNames.push({ lookupName: `component:${node.path.original}`, filePath }); } }, SubExpression(node) { if (node.path.type === 'PathExpression') { - lookupNames.push({ lookupName: `helper:${node.path.original}` }); + lookupNames.push({ lookupName: `helper:${node.path.original}`, filePath }); } }, }); @@ -91,7 +91,7 @@ function findAppName(f) { let code = fs.readFileSync(filePath).toString(); let root = recast.parse(code); - lookupNames = lookupNames.concat(_preparseTemplate(root, lookupNames, filePath)); + lookupNames = lookupNames.concat(_preparseTemplate(root, filePath)); } debug('Gathering telemetry data from %s ...', appLocation); @@ -105,7 +105,7 @@ function findAppName(f) { appName ); - let telemetry = getTelemetry(); + let telemetry = getTelemetry(TELEMETRY_KEY); debug('Gathered telemetry on %d modules', Object.keys(telemetry).length); diff --git a/package.json b/package.json index 6ddf3c4e..05b73ba7 100644 --- a/package.json +++ b/package.json @@ -33,7 +33,7 @@ "dependencies": { "codemod-cli": "^2.1.0", "debug": "^4.1.1", - "ember-codemods-telemetry-helpers": "^1.2.0", + "ember-codemods-telemetry-helpers": "^1.2.1", "ember-template-recast": "^3.3.2" }, "devDependencies": { diff --git a/transforms/no-implicit-this/__testfixtures__/-mock-telemetry.json b/transforms/no-implicit-this/__testfixtures__/-mock-telemetry.json index 6aeb03c0..922e37a3 100644 --- a/transforms/no-implicit-this/__testfixtures__/-mock-telemetry.json +++ b/transforms/no-implicit-this/__testfixtures__/-mock-telemetry.json @@ -1,18 +1,16 @@ { - "telemetry-data": { - "component:handlebars-with-prefix-component-properties-only": { - "type": "Component", - "localProperties": ["baz", "bang"], - "filePath": "transforms/no-implicit-this/__testfixtures__/handlebars-with-prefix-component-properties-only.hbs" - }, - "some-component": { "type": "Component" }, - "my-component": { "type": "Component" }, - "namespace/my-component": { "type": "Component" }, - "block-component": { "type": "Component" }, - "foo": { "type": "Component" }, - "namespace/foo": { "type": "Component" }, - "my-helper": { "type": "Helper" }, - "a-helper": { "type": "Helper" }, - "foo-bar-baz": { "type": "Component" } - } + "component:handlebars-with-prefix-component-properties-only": { + "type": "Component", + "localProperties": ["baz", "bang"], + "filePath": "transforms/no-implicit-this/__testfixtures__/handlebars-with-prefix-component-properties-only.hbs" + }, + "some-component": { "type": "Component" }, + "my-component": { "type": "Component" }, + "namespace/my-component": { "type": "Component" }, + "block-component": { "type": "Component" }, + "foo": { "type": "Component" }, + "namespace/foo": { "type": "Component" }, + "my-helper": { "type": "Helper" }, + "a-helper": { "type": "Helper" }, + "foo-bar-baz": { "type": "Component" } } diff --git a/transforms/no-implicit-this/helpers/plugin.js b/transforms/no-implicit-this/helpers/plugin.js index 92d45350..2af136d8 100644 --- a/transforms/no-implicit-this/helpers/plugin.js +++ b/transforms/no-implicit-this/helpers/plugin.js @@ -6,7 +6,6 @@ const path = require('path'); // sorta. telemetry needs to be defined. // telemtry can be populated with -mock-telemetry.json const KNOWN_HELPERS = require('./known-helpers'); -const { TELEMETRY_KEY } = require('./util'); function getTelemetryObjByName(name, telemetry) { let telemetryLookupName = Object.keys(telemetry).find(item => item.split(':').pop() === name); @@ -19,7 +18,7 @@ function transform(root, options = {}) { let b = recast.builders; let scopedParams = []; - let telemetry = options.telemetry ? options.telemetry[TELEMETRY_KEY] : {}; + let telemetry = options.telemetry ? options.telemetry : {}; let customHelpers = options.customHelpers || []; diff --git a/transforms/no-implicit-this/helpers/util.js b/transforms/no-implicit-this/helpers/util.js index ff93e4ce..46b31405 100644 --- a/transforms/no-implicit-this/helpers/util.js +++ b/transforms/no-implicit-this/helpers/util.js @@ -43,7 +43,7 @@ function detectTypeAndName(entry) { `**/controllers/**/${fileName}.js`, ], { - ignore: ['node_modules/**'], + ignore: ['**/node_modules/**'], } ); if (matched.length) { @@ -52,9 +52,10 @@ function detectTypeAndName(entry) { detectedHelperName = getDetectedName(matchedItem, fileName, 'helper'); detectedControllerName = getDetectedName(matchedItem, fileName, 'controller'); }); + let detectedName = detectedComponentName || detectedHelperName || detectedControllerName; + return { lookupName: detectedName, filePath: entry }; } - let detectedName = detectedComponentName || detectedHelperName || detectedControllerName; - return { lookupName: detectedName, filePath: entry }; + return null; } /** diff --git a/transforms/no-implicit-this/index.js b/transforms/no-implicit-this/index.js index 42a4fd3b..b02a68b9 100644 --- a/transforms/no-implicit-this/index.js +++ b/transforms/no-implicit-this/index.js @@ -5,6 +5,7 @@ const debug = require('debug')('ember-no-implicit-this-codemod:transform'); const recast = require('ember-template-recast'); const { getTelemetry } = require('ember-codemods-telemetry-helpers'); const transform = require('./helpers/plugin'); +const { TELEMETRY_KEY } = require('./helpers/util'); const { getOptions: getCLIOptions } = require('codemod-cli'); const DEFAULT_OPTIONS = {}; @@ -37,7 +38,7 @@ function getOptions() { let options = { noStrict: cliOptions.noStrict, customHelpers: _getCustomHelpersFromConfig(cliOptions.config), - telemetry: getTelemetry(), + telemetry: getTelemetry(TELEMETRY_KEY), }; return options; } diff --git a/transforms/no-implicit-this/test-helpers.js b/transforms/no-implicit-this/test-helpers.js index b82b6b9c..a1b29fe4 100644 --- a/transforms/no-implicit-this/test-helpers.js +++ b/transforms/no-implicit-this/test-helpers.js @@ -1,8 +1,9 @@ -const { setTelemetry } = require('ember-codemods-telemetry-helpers'); +const { setTelemetryWithKey } = require('ember-codemods-telemetry-helpers'); const mockTelemetryData = require('./__testfixtures__/-mock-telemetry.json'); +const { TELEMETRY_KEY } = require('./helpers/util'); function setupTelemetry() { - setTelemetry(mockTelemetryData); + setTelemetryWithKey(TELEMETRY_KEY, mockTelemetryData); } module.exports = { setupTelemetry }; diff --git a/yarn.lock b/yarn.lock index 1781e57c..9373c004 100644 --- a/yarn.lock +++ b/yarn.lock @@ -2266,10 +2266,10 @@ electron-to-chromium@^1.3.295: resolved "https://registry.yarnpkg.com/electron-to-chromium/-/electron-to-chromium-1.3.296.tgz#a1d4322d742317945285d3ba88966561b67f3ac8" integrity sha512-s5hv+TSJSVRsxH190De66YHb50pBGTweT9XGWYu/LMR20KX6TsjFzObo36CjVAzM+PUeeKSBRtm/mISlCzeojQ== -ember-codemods-telemetry-helpers@^1.2.0: - version "1.2.0" - resolved "https://registry.yarnpkg.com/ember-codemods-telemetry-helpers/-/ember-codemods-telemetry-helpers-1.2.0.tgz#d88541ee3f75de3f4094a40ce6a1d09df72dcbb4" - integrity sha512-m7D9pNbGC2EhoZ70xP/Jn5oWcpXFi16J4BKk810bnnGXVxx4Vl2YuKSs+SaAjEFnBIyqU0o4CXeCftBmy4Q7HA== +ember-codemods-telemetry-helpers@^1.2.1: + version "1.2.1" + resolved "https://registry.yarnpkg.com/ember-codemods-telemetry-helpers/-/ember-codemods-telemetry-helpers-1.2.1.tgz#d5ca292605ee8337d537ccb3c6ca0246d76fec95" + integrity sha512-/5G7E8H1BXZVpDWlvgO1QrtqDKuYGuf6O+ne6akWowdXa59oMXRaw6nGN4ZS8WItOlX1aOB2o6sYy3V75Y86aQ== dependencies: fs-extra "^8.1.0" git-repo-info "^2.1.0" From f42215b106d8ca7c2fdf9dd0ecfb83d05dd9139c Mon Sep 17 00:00:00 2001 From: Suchita Doshi Date: Fri, 24 Apr 2020 11:13:52 -0700 Subject: [PATCH 6/6] change the flag name --- ...andlebars-with-prefix-component-properties-only.options.json | 2 +- transforms/no-implicit-this/helpers/plugin.js | 2 +- transforms/no-implicit-this/index.js | 2 +- 3 files changed, 3 insertions(+), 3 deletions(-) diff --git a/transforms/no-implicit-this/__testfixtures__/handlebars-with-prefix-component-properties-only.options.json b/transforms/no-implicit-this/__testfixtures__/handlebars-with-prefix-component-properties-only.options.json index 18dcf8f4..3f94c5e8 100644 --- a/transforms/no-implicit-this/__testfixtures__/handlebars-with-prefix-component-properties-only.options.json +++ b/transforms/no-implicit-this/__testfixtures__/handlebars-with-prefix-component-properties-only.options.json @@ -1,3 +1,3 @@ { - "noStrict": "true" + "prefixComponentPropertiesOnly": "true" } diff --git a/transforms/no-implicit-this/helpers/plugin.js b/transforms/no-implicit-this/helpers/plugin.js index 2af136d8..f4f95c5d 100644 --- a/transforms/no-implicit-this/helpers/plugin.js +++ b/transforms/no-implicit-this/helpers/plugin.js @@ -73,7 +73,7 @@ function transform(root, options = {}) { // check for the flag for stricter prefixing. This check ensures that it only // prefixes `this` to the properties owned by the backing JS class of the template. - if (options.noStrict === 'true') { + if (options.prefixComponentPropertiesOnly === 'true') { const matchedFilePath = Object.keys(telemetry).find( item => telemetry[item].filePath === path.relative(process.cwd(), options.filePath) ); diff --git a/transforms/no-implicit-this/index.js b/transforms/no-implicit-this/index.js index b02a68b9..54b4185c 100644 --- a/transforms/no-implicit-this/index.js +++ b/transforms/no-implicit-this/index.js @@ -36,7 +36,7 @@ function _getCustomHelpersFromConfig(configPath) { function getOptions() { let cliOptions = getCLIOptions(); let options = { - noStrict: cliOptions.noStrict, + prefixComponentPropertiesOnly: cliOptions.prefixComponentPropertiesOnly, customHelpers: _getCustomHelpersFromConfig(cliOptions.config), telemetry: getTelemetry(TELEMETRY_KEY), };