diff --git a/bin/cli.js b/bin/cli.js index e0ab4b39..181bf062 100755 --- a/bin/cli.js +++ b/bin/cli.js @@ -2,19 +2,111 @@ 'use strict'; const debug = require('debug')('ember-no-implicit-this-codemod'); +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 { - gatherTelemetryForUrl, - analyzeEmberObject, - getTelemetry, -} = require('ember-codemods-telemetry-helpers'); + 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); +/** + * 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, filePath) { + let lookupNames = []; + recast.traverse(root, { + MustacheStatement(node) { + if (node.path.type === 'PathExpression') { + 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}`, filePath }); + } + }, + + SubExpression(node) { + if (node.path.type === 'PathExpression') { + lookupNames.push({ lookupName: `helper:${node.path.original}`, filePath }); + } + }, + }); + 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')) { + let emberAddon = fileName['ember-addon']; + // There could be cases where the root package.json might have multiple Ember apps within. + return emberAddon && emberAddon.apps ? emberAddon.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, filePath)); + } + debug('Gathering telemetry data from %s ...', appLocation); - await gatherTelemetryForUrl(appLocation, analyzeEmberObject); - let telemetry = getTelemetry(); + // This is for collecting metadata for the app just once to generate the map of lookupnames to local properties + await gatherSingleTelemetryForUrl( + appLocation, + { telemetryKey: TELEMETRY_KEY }, + appResolver, + lookupNames, + appName + ); + + let telemetry = getTelemetry(TELEMETRY_KEY); + 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..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.1.0", + "ember-codemods-telemetry-helpers": "^1.2.1", "ember-template-recast": "^3.3.2" }, "devDependencies": { @@ -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..922e37a3 100644 --- a/transforms/no-implicit-this/__testfixtures__/-mock-telemetry.json +++ b/transforms/no-implicit-this/__testfixtures__/-mock-telemetry.json @@ -1,4 +1,9 @@ { + "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" }, 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..3f94c5e8 --- /dev/null +++ b/transforms/no-implicit-this/__testfixtures__/handlebars-with-prefix-component-properties-only.options.json @@ -0,0 +1,3 @@ +{ + "prefixComponentPropertiesOnly": "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..f4f95c5d 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 : {}; 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.prefixComponentPropertiesOnly === '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..46b31405 --- /dev/null +++ b/transforms/no-implicit-this/helpers/util.js @@ -0,0 +1,117 @@ +const path = require('path'); +const globby = require('globby'); +const TELEMETRY_KEY = 'telemetry-data'; + +/** + * 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 }; + } + return null; +} + +/** + * 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, TELEMETRY_KEY }; diff --git a/transforms/no-implicit-this/index.js b/transforms/no-implicit-this/index.js index 1bd55ff1..54b4185c 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 = {}; @@ -35,15 +36,16 @@ function _getCustomHelpersFromConfig(configPath) { function getOptions() { let cliOptions = getCLIOptions(); let options = { + prefixComponentPropertiesOnly: cliOptions.prefixComponentPropertiesOnly, customHelpers: _getCustomHelpersFromConfig(cliOptions.config), - telemetry: getTelemetry(), + telemetry: getTelemetry(TELEMETRY_KEY), }; return options; } 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..a1b29fe4 100644 --- a/transforms/no-implicit-this/test-helpers.js +++ b/transforms/no-implicit-this/test-helpers.js @@ -1,18 +1,9 @@ -const path = require('path'); -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() { - let mockTelemetry = {}; - - Object.keys(mockTelemetryData).forEach(key => { - let value = mockTelemetryData[key] || {}; - let mockPath = path.resolve(__dirname, `./__testfixtures__/${key}`); - - mockTelemetry[mockPath] = value; - }); - - setTelemetry(mockTelemetry); + setTelemetryWithKey(TELEMETRY_KEY, mockTelemetryData); } module.exports = { setupTelemetry }; diff --git a/yarn.lock b/yarn.lock index 4b6e11ca..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.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.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" @@ -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"