Skip to content

Add support for prefixing this only for owned properties #123

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
wants to merge 6 commits into
base: main
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
104 changes: 98 additions & 6 deletions bin/cli.js
Original file line number Diff line number Diff line change
Expand Up @@ -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');
Expand Down
3 changes: 2 additions & 1 deletion package.json
Original file line number Diff line number Diff line change
Expand Up @@ -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": {
Expand All @@ -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",
Expand Down
Original file line number Diff line number Diff line change
@@ -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" },
Expand Down
Original file line number Diff line number Diff line change
@@ -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'}}
Original file line number Diff line number Diff line change
@@ -0,0 +1,3 @@
{
"prefixComponentPropertiesOnly": "true"
}
Original file line number Diff line number Diff line change
@@ -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'}}
62 changes: 32 additions & 30 deletions transforms/no-implicit-this/helpers/plugin.js
Original file line number Diff line number Diff line change
@@ -1,20 +1,24 @@
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
*/
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 || [];

Expand Down Expand Up @@ -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);
Expand All @@ -89,21 +111,21 @@ 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;
}

return false;
}

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;
}

Expand Down Expand Up @@ -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;
117 changes: 117 additions & 0 deletions transforms/no-implicit-this/helpers/util.js
Original file line number Diff line number Diff line change
@@ -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 };
Loading