Skip to content
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

Minor improvements from a few more translation examples #90

Merged
merged 3 commits into from
May 20, 2016
Merged
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
8 changes: 8 additions & 0 deletions packages/esify/README.md
Original file line number Diff line number Diff line change
Expand Up @@ -48,5 +48,13 @@ module.exports = {
properties: ['spy', 'stub', 'mock', 'server', 'requests'],
},
},

// A list of globals and their associated import paths for global-identifier-to-import
globalIdentifiers: {
_: 'lodash',
$: 'jquery',
jQuery: 'jquery',
moment: 'moment',
},
}
```
24 changes: 21 additions & 3 deletions packages/esify/index.js
Original file line number Diff line number Diff line change
Expand Up @@ -7,14 +7,13 @@ require('babel-register')({ignore: false});
var TRANSFORMS = [
{path: 'shopify-codemod/transforms/coffeescript-soak-to-condition'},
{path: 'shopify-codemod/transforms/ternary-statement-to-if-statement'},
{path: 'shopify-codemod/transforms/remove-useless-return-from-test', test: true},
{path: 'shopify-codemod/transforms/mocha-context-to-closure', test: true},
{path: 'shopify-codemod/transforms/mocha-context-to-global-reference', test: true},
{path: 'shopify-codemod/transforms/coffeescript-range-output-to-helper'},
{path: 'shopify-codemod/transforms/remove-useless-return-from-test', test: true},
{path: 'shopify-codemod/transforms/remove-addeventlistener-returns'},
{path: 'shopify-codemod/transforms/conditional-assign-to-if-statement'},
{path: 'shopify-codemod/transforms/global-assignment-to-default-export', test: false},
{path: 'shopify-codemod/transforms/global-reference-to-import'},
// Order is significant for these initial assert transforms; think carefully before reordering.
{path: 'shopify-codemod/transforms/assert/assert-false-to-assert-fail', test: true},
{path: 'shopify-codemod/transforms/assert/assert-to-assert-ok', test: true},
Expand All @@ -29,14 +28,18 @@ var TRANSFORMS = [
{path: 'shopify-codemod/transforms/assert/falsy-called-method-to-assert-not-called', test: true},
// These are more generic, stylistic transforms, so they should come last to catch any
// new nodes introduced by other transforms
{path: 'shopify-codemod/transforms/constant-function-expression-to-statement'},
{path: 'shopify-codemod/transforms/remove-empty-returns'},
{path: 'shopify-codemod/transforms/function-to-arrow'},
{path: 'js-codemod/transforms/arrow-function'},
{path: 'js-codemod/transforms/template-literals'},
{path: 'shopify-codemod/transforms/strip-template-literal-parenthesis'},
{path: 'js-codemod/transforms/object-shorthand'},
// constant-function-expression-to-statement and global-reference-to-import need
// `const` references, so they must happen after `no-vars`
{path: 'js-codemod/transforms/no-vars'},
{path: 'shopify-codemod/transforms/constant-function-expression-to-statement'},
{path: 'shopify-codemod/transforms/global-reference-to-import'},
{path: 'shopify-codemod/transforms/global-identifier-to-import'},
{path: 'js-codemod/transforms/unquote-properties'},
];

Expand Down Expand Up @@ -64,6 +67,21 @@ function loadOptions() {
properties: ['spy', 'stub', 'mock', 'server', 'requests'],
},
},
globalIdentifiers: {
_: 'lodash',
$: 'jquery',
jQuery: 'jquery',
moment: 'moment',
jstz: 'jstimezonedetect',
mousetrap: 'mousetrap',
URI: 'urijs',
URITemplate: 'urijs/src/URITemplate',
ReconnectingWebSocket: 'shopify-reconnecting-websocket',
d3: 'd3',
NProgress: 'NProgress',
FastClick: 'shopify-fastclick',
Clipboard: 'clipboard',
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

nice 👍

},
};
}
}
Expand Down
23 changes: 23 additions & 0 deletions packages/shopify-codemod/README.md
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,29 @@ This repository contains a collection of Codemods written with [JSCodeshift](htt

## Included Transforms

### `global-identifer-to-import`

Creates import statements for global identifiers. Use the `globalIdentifiers` option to specify identifier/ import path pairs.

```sh
jscodeshift -t shopify-codemods/transforms/global-identifier-to-import <file>
```

#### Example

```js
// with {globalIdentifiers: {_: 'lodash', $: 'jquery'}}
_.map([], _.identity);
window.$('.foo').find('.bar');

// BECOMES:

import _ from 'lodash';
import $ from 'jquery';
_.map([], _.identity);
$('.foo').find('.bar');
```

### `coffeescript-soak-to-condition`

Changes the output of CoffeeScript’s soak operations (`foo?.bar.baz?()`) into a cleaner, more idiomatic JavaScript expression appropriate for its location in code.
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,7 @@
'expose Foo.Bar';

[].map(_.identity);
window.moment.fromDate(Date.now());
foo.$.find('.bar');
window.React.renderToString(<div />);
jstz(Date.now());
Original file line number Diff line number Diff line change
@@ -0,0 +1,11 @@
'expose Foo.Bar';

import jstz from 'jstimezone/jstz';
import moment from 'moment';
import _ from 'lodash';

[].map(_.identity);
moment.fromDate(Date.now());
foo.$.find('.bar');
window.React.renderToString(<div />);
jstz(Date.now());
Original file line number Diff line number Diff line change
@@ -0,0 +1,25 @@
suite('a', () => {
beforeEach(function() {
return (function() {
for (var i in [1, 2, 3]) {
assert(typeof i === 'number');
}
})();
});

afterEach(() => {
return (() => {
for (var i in [1, 2, 3]) {
assert(typeof i === 'number');
}
})();
});

it(() => {
return ((i) => {
for (var i in [1, 2, 3]) {
assert(typeof i === 'number');
}
})(i);
});
});
Original file line number Diff line number Diff line change
@@ -0,0 +1,21 @@
suite('a', () => {
beforeEach(function() {
for (var i in [1, 2, 3]) {
assert(typeof i === 'number');
}
});

afterEach(() => {
for (var i in [1, 2, 3]) {
assert(typeof i === 'number');
}
});

it(() => {
((i) => {
for (var i in [1, 2, 3]) {
assert(typeof i === 'number');
}
})(i);
});
});
Original file line number Diff line number Diff line change
@@ -0,0 +1,14 @@
import 'test-helper';
import globalIdentifierToImport from 'global-identifier-to-import';

describe('globalIdentifierToImport', () => {
it('transforms basic setup variables', () => {
expect(globalIdentifierToImport).to.transform('global-identifier-to-import/basic', {
globalIdentifiers: {
_: 'lodash',
moment: 'moment',
jstz: 'jstimezone/jstz',
},
});
});
});
Original file line number Diff line number Diff line change
Expand Up @@ -5,4 +5,8 @@ describe('removeUselessReturnFromTest', () => {
it('transforms basic setup variables', () => {
expect(removeUselessReturnFromTest).to.transform('remove-useless-return-from-test/basic');
});

it('transforms empty returned IIFEs', () => {
expect(removeUselessReturnFromTest).to.transform('remove-useless-return-from-test/iife');
});
});
Original file line number Diff line number Diff line change
@@ -1,10 +1,8 @@
import {matchLast as matchLastNode} from './utils';
import {matchLast} from './utils';

const INCLUSIVE_OPERATORS = new Set(['<=', '>=']);

export default function coffeescriptRangeOutputToHelper({source}, {jscodeshift: j}, {printOptions = {}}) {
const matchLast = matchLastNode.bind(null, j);

function isCoffeeScriptRangeFunctionBody(statements) {
const lastStatement = statements[statements.length - 1];
const isProperReturn = j.match(lastStatement, {
Expand Down
49 changes: 49 additions & 0 deletions packages/shopify-codemod/transforms/global-identifier-to-import.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,49 @@
import {insertAfterDirectives} from './utils';

export default function globalIdentifierToImport({source}, {jscodeshift: j}, {printOptions = {}, globalIdentifiers = {}}) {
return j(source)
.find(j.Program)
.forEach((path) => {
const imports = new Set();

function isGlobalIdentifierName(name) {
return globalIdentifiers.hasOwnProperty(name);
}

function isWindow(windowPath) {
return windowPath.get('name').value === 'window' && !isNestedInMemberExpression(windowPath);
}

function isNestedInMemberExpression(aPath) {
const parentNode = aPath.parentPath.node;
return j.MemberExpression.check(parentNode) && parentNode.property === aPath.node;
}

function isGlobalIdentifier(identifierPath) {
return !isNestedInMemberExpression(identifierPath) || isWindow(identifierPath.parentPath.get('object'));
}

j(path)
.find(j.Identifier, {name: isGlobalIdentifierName})
.filter(isGlobalIdentifier)
.forEach((identifierPath) => {
imports.add(identifierPath.node.name);

// can only happen for window.globalIdentifier
if (isNestedInMemberExpression(identifierPath)) {
identifierPath.parentPath.replace(identifierPath.node);
}
});

const {node: {body}} = path;
for (const anImport of imports.values()) {
insertAfterDirectives(
body,
j.importDeclaration([
j.importDefaultSpecifier(j.identifier(anImport)),
], j.literal(globalIdentifiers[anImport]))
);
}
})
.toSource(printOptions);
}
16 changes: 12 additions & 4 deletions packages/shopify-codemod/transforms/global-reference-to-import.js
Original file line number Diff line number Diff line change
Expand Up @@ -23,6 +23,7 @@ export default function globalReferenceToImport(
appGlobalIdentifiers,
}) {
const [binary, args] = determineFileSearcher();
const fileForIdentifier = {};

/*
* findDeclaringFile uses ack or the_silver_searcher to find the file in which something is declared.
Expand Down Expand Up @@ -53,6 +54,15 @@ export default function globalReferenceToImport(
return relative(absolutePath, files[0]).replace(/\.[a-z]+$/, '');
}

function getDeclaringFile(identifier) {
if (!fileForIdentifier.hasOwnProperty(identifier)) {
fileForIdentifier[identifier] = findDeclaringFile(identifier);
}

return fileForIdentifier[identifier];
}


function isGlobalReference(object) {
return appGlobalIdentifiers.indexOf(findFirstMember(object).name) >= 0;
}
Expand All @@ -67,10 +77,8 @@ export default function globalReferenceToImport(
if (imports.has(member)) {
return imports.get(member).name;
} else {
const file = findDeclaringFile(member);
if (file === null) {
return null;
}
const file = getDeclaringFile(member);
if (file === null) { return null; }
const name = findLastMember(node).name;
imports.set(member, {file, name});
return name;
Expand Down
Original file line number Diff line number Diff line change
@@ -1,13 +1,6 @@
import {MOCHA_FUNCTIONS} from './utils';

export default function mochaContextToGlobalReference(
{source},
{jscodeshift: j},
{
printOptions = {},
testContextToGlobals = {},
}
) {
export default function mochaContextToGlobalReference({source}, {jscodeshift: j}, {printOptions = {}, testContextToGlobals = {}}) {
const propMapping = Object.keys(testContextToGlobals).reduce((map, testGlobal) => {
const testGlobalDetails = testContextToGlobals[testGlobal];
testGlobalDetails.properties.forEach((prop) => {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,7 @@ export default function removeEmptyReturns({source}, {jscodeshift: j}, {printOpt
return j(source)
.find(j.Function, {
body: {
body: matchLast(j, {
body: matchLast({
type: 'ReturnStatement',
argument: null,
}),
Expand Down
Original file line number Diff line number Diff line change
@@ -1,7 +1,19 @@
import {MOCHA_FUNCTIONS, matchLast as matchLastNode} from './utils';
import {MOCHA_FUNCTIONS, matchLast} from './utils';

export default function removeUselessReturnFromTest({source}, {jscodeshift: j}, {printOptions = {}}) {
const matchLast = matchLastNode.bind(null, j);
function isUselessIIFE(node) {
return j.match(node, {
type: j.CallExpression.name,
callee: {
type: isFunctionExpressionType,
params: (params) => params.length === 0,
},
});
}

function isFunctionExpressionType(type) {
return type === j.ArrowFunctionExpression.name || type === j.FunctionExpression.name;
}

return j(source)
.find(j.CallExpression, {
Expand All @@ -10,11 +22,11 @@ export default function removeUselessReturnFromTest({source}, {jscodeshift: j},
name: (name) => MOCHA_FUNCTIONS.has(name),
},
arguments: matchLast({
type: (type) => type === 'ArrowFunctionExpression' || type === 'FunctionExpression',
type: isFunctionExpressionType,
body: {
type: 'BlockStatement',
type: j.BlockStatement.name,
body: matchLast({
type: 'ReturnStatement',
type: j.ReturnStatement.name,
}),
},
}),
Expand All @@ -23,7 +35,11 @@ export default function removeUselessReturnFromTest({source}, {jscodeshift: j},
const {body: {body}} = path.node.arguments[path.node.arguments.length - 1];
const returnStatement = body[body.length - 1];
if (returnStatement.argument) {
body[body.length - 1] = j.expressionStatement(returnStatement.argument);
if (isUselessIIFE(returnStatement.argument)) {
returnStatement.argument.callee.body.body.forEach((statement) => { body[body.length - 1] = statement; });
} else {
body[body.length - 1] = j.expressionStatement(returnStatement.argument);
}
} else {
body.pop();
}
Expand Down
4 changes: 3 additions & 1 deletion packages/shopify-codemod/transforms/utils.js
Original file line number Diff line number Diff line change
@@ -1,3 +1,5 @@
import j from 'jscodeshift';

export function findFirstMember(node) {
if (node.type === 'MemberExpression') {
return findFirstMember(node.object);
Expand All @@ -12,7 +14,7 @@ export function findLastMember(node) {
return node;
}

export function matchLast(j, matcher) {
export function matchLast(matcher) {
return (nodes) => nodes.length > 0 && j.match(nodes[nodes.length - 1], matcher);
}

Expand Down