Skip to content

Commit 702dd40

Browse files
author
Suchita Doshi
committed
Add support for prefixing this only for owned properties
1 parent 6cd070a commit 702dd40

11 files changed

+282
-57
lines changed

bin/cli.js

+87-6
Original file line numberDiff line numberDiff line change
@@ -2,19 +2,100 @@
22
'use strict';
33

44
const debug = require('debug')('ember-no-implicit-this-codemod');
5-
const {
6-
gatherTelemetryForUrl,
7-
analyzeEmberObject,
8-
getTelemetry,
9-
} = require('ember-codemods-telemetry-helpers');
5+
const globby = require('globby');
6+
const finder = require('find-package-json');
7+
const recast = require('ember-template-recast');
8+
const fs = require('fs');
9+
const path = require('path');
10+
const { appResolver, detectTypeAndName } = require('../transforms/no-implicit-this/helpers/util');
11+
const { gatherSingleTelemetryForUrl, getTelemetry } = require('ember-codemods-telemetry-helpers');
1012
const appLocation = process.argv[2];
1113
const args = process.argv.slice(3);
1214

15+
/**
16+
* Pre parse the template to collect information for potential helper/components
17+
* We are pushing the lookup names for any `PathExpression` occuring in:
18+
* - MustacheStatement: It could be helper or component
19+
* - BlockStatement: It can only be a component
20+
* - SubExpression: It can only be a helper
21+
* The values from this lookup array will be consumed by the app's resolver.
22+
* If the lookup name is is found in the app's registry, it would help
23+
* determine local properties for a given template's backing js class.
24+
* @param {*} root
25+
* @param {*} lookupNames
26+
*/
27+
function _preparseTemplate(root) {
28+
let lookupNames = [];
29+
recast.traverse(root, {
30+
MustacheStatement(node) {
31+
if (node.path.type === 'PathExpression') {
32+
lookupNames.push({ lookupName: `component:${node.path.original}` });
33+
lookupNames.push({ lookupName: `helper:${node.path.original}` });
34+
}
35+
},
36+
37+
BlockStatement(node) {
38+
if (node.path.type === 'PathExpression') {
39+
lookupNames.push({ lookupName: `component:${node.path.original}` });
40+
}
41+
},
42+
43+
SubExpression(node) {
44+
if (node.path.type === 'PathExpression') {
45+
lookupNames.push({ lookupName: `helper:${node.path.original}` });
46+
}
47+
},
48+
});
49+
return lookupNames;
50+
}
51+
52+
/**
53+
* Return the app name based on the package json, keep calling the function
54+
* recursively until you reach the root of the app.
55+
* @param {*} f
56+
*/
57+
function findAppName(f) {
58+
let fileName = f.next().value;
59+
if (fileName.keywords && fileName.keywords.includes('ember-addon')) {
60+
return findAppName(f);
61+
} else if (Object.keys(fileName.devDependencies).includes('ember-cli')) {
62+
// There could be cases where the root package.json might have multiple Ember apps within.
63+
return fileName['ember-addon'].apps ? fileName['ember-addon'].apps : [fileName.name];
64+
}
65+
}
66+
1367
(async () => {
68+
const filePaths = globby.sync(args[0], { ignore: 'node_modules/**' });
69+
70+
// Get the package.json for the first file path and pass it to the `findAppName` function.
71+
// Note: We just need the first found file path since from there we would be able
72+
// to get the root level app name.
73+
const appName = filePaths ? findAppName(finder(filePaths[0])) : null;
74+
75+
let lookupNames = filePaths.map(detectTypeAndName).filter(item => item !== null);
76+
// Pre-parse the each template file.
77+
for (let i = 0; i < filePaths.length; i++) {
78+
let filePath = filePaths[i];
79+
let extension = path.extname(filePath);
80+
81+
if (!['.hbs'].includes(extension.toLowerCase())) {
82+
// do nothing on non-hbs files
83+
continue;
84+
}
85+
86+
let code = fs.readFileSync(filePath).toString();
87+
let root = recast.parse(code);
88+
89+
lookupNames = lookupNames.concat(_preparseTemplate(root, lookupNames, filePath));
90+
}
91+
1492
debug('Gathering telemetry data from %s ...', appLocation);
15-
await gatherTelemetryForUrl(appLocation, analyzeEmberObject);
93+
94+
// This is for collecting metadata for the app just once to generate the map of lookupnames to local properties
95+
await gatherSingleTelemetryForUrl(appLocation, appResolver, lookupNames, appName);
1696

1797
let telemetry = getTelemetry();
98+
1899
debug('Gathered telemetry on %d modules', Object.keys(telemetry).length);
19100

20101
require('codemod-cli').runTransform(__dirname, 'no-implicit-this', args, 'hbs');

package.json

+1
Original file line numberDiff line numberDiff line change
@@ -49,6 +49,7 @@
4949
"eslint-config-prettier": "^6.9.0",
5050
"eslint-plugin-prettier": "^3.1.2",
5151
"execa": "^3.4.0",
52+
"find-package-json": "^1.2.0",
5253
"jest": "^24.9.0",
5354
"prettier": "^1.19.1",
5455
"release-it": "^12.4.2",
Original file line numberDiff line numberDiff line change
@@ -1,11 +1,18 @@
11
{
2-
"some-component": { "type": "Component" },
3-
"my-component": { "type": "Component" },
4-
"namespace/my-component": { "type": "Component" },
5-
"block-component": { "type": "Component" },
6-
"foo": { "type": "Component" },
7-
"namespace/foo": { "type": "Component" },
8-
"my-helper": { "type": "Helper" },
9-
"a-helper": { "type": "Helper" },
10-
"foo-bar-baz": { "type": "Component" }
2+
"single-telemetry": {
3+
"component:handlebars-with-prefix-component-properties-only": {
4+
"type": "Component",
5+
"localProperties": ["baz", "bang"],
6+
"filePath": "transforms/no-implicit-this/__testfixtures__/handlebars-with-prefix-component-properties-only.hbs"
7+
},
8+
"some-component": { "type": "Component" },
9+
"my-component": { "type": "Component" },
10+
"namespace/my-component": { "type": "Component" },
11+
"block-component": { "type": "Component" },
12+
"foo": { "type": "Component" },
13+
"namespace/foo": { "type": "Component" },
14+
"my-helper": { "type": "Helper" },
15+
"a-helper": { "type": "Helper" },
16+
"foo-bar-baz": { "type": "Component" }
17+
}
1118
}
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,10 @@
1+
{{my-component "string"}}
2+
{{my-component 1}}
3+
{{my-component foo}}
4+
{{my-component baz}}
5+
{{my-component bang}}
6+
{{my-component @foo}}
7+
{{my-component property}}
8+
{{my-component (my-helper baz)}}
9+
{{my-component (my-helper 1)}}
10+
{{get this 'key'}}
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,3 @@
1+
{
2+
"noStrict": "true"
3+
}
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,10 @@
1+
{{my-component "string"}}
2+
{{my-component 1}}
3+
{{my-component foo}}
4+
{{my-component this.baz}}
5+
{{my-component this.bang}}
6+
{{my-component @foo}}
7+
{{my-component property}}
8+
{{my-component (my-helper this.baz)}}
9+
{{my-component (my-helper 1)}}
10+
{{get this 'key'}}

transforms/no-implicit-this/helpers/plugin.js

+32-30
Original file line numberDiff line numberDiff line change
@@ -1,20 +1,24 @@
11
const debug = require('debug')('ember-no-implicit-this-codemod:plugin');
22
const recast = require('ember-template-recast');
3+
const path = require('path');
34

45
// everything is copy-pasteable to astexplorer.net.
56
// sorta. telemetry needs to be defined.
67
// telemtry can be populated with -mock-telemetry.json
78
const KNOWN_HELPERS = require('./known-helpers');
89

10+
function getTelemetryObjByName(name, telemetry) {
11+
let telemetryLookupName = Object.keys(telemetry).find(item => item.split(':').pop() === name);
12+
return telemetry[telemetryLookupName] || {};
13+
}
914
/**
1015
* plugin entrypoint
1116
*/
1217
function transform(root, options = {}) {
1318
let b = recast.builders;
1419

1520
let scopedParams = [];
16-
let telemetry = options.telemetry || {};
17-
let [components, helpers] = populateInvokeables(telemetry);
21+
let telemetry = options.telemetry ? options.telemetry['single-telemetry'] : {};
1822

1923
let customHelpers = options.customHelpers || [];
2024

@@ -67,6 +71,24 @@ function transform(root, options = {}) {
6771
return;
6872
}
6973

74+
// check for the flag for stricter prefixing. This check ensures that it only
75+
// prefixes `this` to the properties owned by the backing JS class of the template.
76+
if (options.noStrict === 'true') {
77+
const matchedFilePath = Object.keys(telemetry).find(
78+
item => telemetry[item].filePath === path.relative(process.cwd(), options.filePath)
79+
);
80+
81+
if (matchedFilePath) {
82+
let lookupObject = telemetry[matchedFilePath];
83+
const ownProperties = lookupObject ? lookupObject.localProperties : [];
84+
85+
if (!ownProperties.includes(firstPart)) {
86+
debug(`Skipping \`%s\` because it is not a local property`, node.original);
87+
return;
88+
}
89+
}
90+
}
91+
7092
// skip `hasBlock` keyword
7193
if (node.original === 'hasBlock') {
7294
debug(`Skipping \`%s\` because it is a keyword`, node.original);
@@ -89,21 +111,21 @@ function transform(root, options = {}) {
89111
return true;
90112
}
91113

92-
let helper = helpers.find(path => path.endsWith(`/${name}`));
93-
if (helper) {
94-
let message = `Skipping \`%s\` because it appears to be a helper from the telemetry data: %s`;
95-
debug(message, name, helper);
114+
const telemetryObj = getTelemetryObjByName(name, telemetry);
115+
if (telemetryObj.type === 'Helper') {
116+
let message = `Skipping \`%s\` because it appears to be a helper from the lookup object`;
117+
debug(message, name);
96118
return true;
97119
}
98120

99121
return false;
100122
}
101123

102124
function isComponent(name) {
103-
let component = components.find(path => path.endsWith(`/${name}`));
104-
if (component) {
105-
let message = `Skipping \`%s\` because it appears to be a component from the telemetry data: %s`;
106-
debug(message, name, component);
125+
const telemetryObj = getTelemetryObjByName(name, telemetry);
126+
if (telemetryObj.type === 'Component') {
127+
let message = `Skipping \`%s\` because it appears to be a component from the lookup object`;
128+
debug(message, name);
107129
return true;
108130
}
109131

@@ -189,24 +211,4 @@ function transform(root, options = {}) {
189211
});
190212
}
191213

192-
function populateInvokeables(telemetry) {
193-
let components = [];
194-
let helpers = [];
195-
196-
for (let name of Object.keys(telemetry)) {
197-
let entry = telemetry[name];
198-
199-
switch (entry.type) {
200-
case 'Component':
201-
components.push(name);
202-
break;
203-
case 'Helper':
204-
helpers.push(name);
205-
break;
206-
}
207-
}
208-
209-
return [components, helpers];
210-
}
211-
212214
module.exports = transform;
+115
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,115 @@
1+
const path = require('path');
2+
const globby = require('globby');
3+
4+
/**
5+
* Generates a lookup name for a backing js class.
6+
* @param {*} matchedItem
7+
* @param {*} fileName
8+
* @param {*} type
9+
*/
10+
function getDetectedName(matchedItem, fileName, type) {
11+
let detectedName = null;
12+
// create regex string to derive the potential addon name and generated the
13+
// lookup name.
14+
let regexString = `(.*)/(addon|app)/${type}s(.*)/${fileName}.js`;
15+
let matchedRegex = matchedItem.match(regexString);
16+
if (matchedRegex) {
17+
const folderType = matchedRegex[2];
18+
const rootName = matchedRegex[1].split('/').pop();
19+
detectedName =
20+
folderType === 'addon' ? `${rootName}@${type}:${fileName}` : `${type}:${fileName}`;
21+
}
22+
return detectedName;
23+
}
24+
25+
/**
26+
* Returns lookup name for a given file path (template file) by searching for a backing
27+
* js file for the template.
28+
* @param {*} entry
29+
*/
30+
function detectTypeAndName(entry) {
31+
let detectedComponentName = null;
32+
let detectedHelperName = null;
33+
let detectedControllerName = null;
34+
const fileName = path.basename(entry).split('.')[0];
35+
// Since we care about the component and helpers (as we do not want to prefix `this`) and
36+
// also we would need to generate lookupNames for controllers and components pertaining to the
37+
// current template file, do a globby search and gather filepaths that match these folders.
38+
const matched = globby.sync(
39+
[
40+
`**/components/**/${fileName}.js`,
41+
`**/helpers/**/${fileName}.js`,
42+
`**/controllers/**/${fileName}.js`,
43+
],
44+
{
45+
ignore: ['node_modules/**'],
46+
}
47+
);
48+
if (matched.length) {
49+
matched.forEach(matchedItem => {
50+
detectedComponentName = getDetectedName(matchedItem, fileName, 'component');
51+
detectedHelperName = getDetectedName(matchedItem, fileName, 'helper');
52+
detectedControllerName = getDetectedName(matchedItem, fileName, 'controller');
53+
});
54+
}
55+
let detectedName = detectedComponentName || detectedHelperName || detectedControllerName;
56+
return { lookupName: detectedName, filePath: entry };
57+
}
58+
59+
/**
60+
* Analyzes the app and collects local properties and type of the lookup name.
61+
* Returns the map of lookupName to its metadata.
62+
* {
63+
* "component:foo": { localProperties: ['foo', 'bar'], type: 'Component', filePath: 'app/components/foo.js' }
64+
* }
65+
* @param {*} lookupNames
66+
* @param {*} appname
67+
*/
68+
function appResolver(lookupNames, currAppName) {
69+
let mapping = {};
70+
if (Array.isArray(currAppName)) {
71+
currAppName.forEach(appItem => {
72+
try {
73+
let app = require(`${appItem}/app`).default.create({ autoboot: false });
74+
let localProperties = [];
75+
lookupNames.forEach(item => {
76+
// Resolve the class from the lookup name, if found, then check if its a helper, component
77+
// or controller and add the local properties & the type to the map.
78+
let klass = app.resolveRegistration(item.lookupName);
79+
if (klass) {
80+
if (klass.proto) {
81+
const protoInfo = klass.proto();
82+
localProperties = Object.keys(protoInfo).filter(
83+
key => !['_super', 'actions'].includes(key)
84+
);
85+
/* globals Ember */
86+
// Determine the type of the class's instance.
87+
let klassType = null;
88+
if (protoInfo instanceof Ember['Controller']) {
89+
klassType = 'Controller';
90+
} else if (protoInfo instanceof Ember['Component']) {
91+
klassType = 'Component';
92+
}
93+
94+
// Create a map with lookupName as key with meta information.
95+
mapping[item.lookupName] = {
96+
filePath: item.filePath,
97+
localProperties,
98+
type: klassType,
99+
};
100+
} else if (klass.isHelperFactory) {
101+
mapping[item.lookupName] = { type: 'Helper', filePath: item.filePath };
102+
}
103+
}
104+
});
105+
app.destroy();
106+
} catch (e) {
107+
console.log(e);
108+
}
109+
});
110+
}
111+
112+
return mapping;
113+
}
114+
115+
module.exports = { appResolver, detectTypeAndName };

0 commit comments

Comments
 (0)