Skip to content

Refactor for more type safety #11

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.

Sign up for GitHub

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

Merged
merged 8 commits into from
Jan 14, 2018
Merged

Refactor for more type safety #11

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
8 commits
Select commit Hold shift + click to select a range
8c63c72
Avoid 'any' for formatMessage
seanf Jan 14, 2018
7d69292
Improve some names
seanf Jan 14, 2018
bdfd32e
Clean up dead var
seanf Jan 14, 2018
485a4ef
Split JSX from method calls
seanf Jan 14, 2018
669db04
Split up findProps
seanf Jan 14, 2018
7648014
Clean up types more
seanf Jan 14, 2018
4b9bbf0
Allow string keys as well as identifiers
seanf Jan 14, 2018
a08c53e
Add tslint and clean up
seanf Jan 14, 2018
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
223 changes: 119 additions & 104 deletions index.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,176 +1,191 @@
import ts = require("typescript");

function isDefineMessages(el: ts.Declaration, tagName: string): el is ts.VariableDeclaration {
function isMethodCall(el: ts.Declaration, methodName: string): el is ts.VariableDeclaration {
return (
ts.isVariableDeclaration(el) &&
el.initializer &&
ts.isCallExpression(el.initializer) &&
el.initializer.expression &&
ts.isIdentifier(el.initializer.expression) &&
el.initializer.expression.text === tagName
el.initializer.expression.text === methodName
);
}

// Should be pretty fast: https://stackoverflow.com/a/34491287/14379
// tslint:disable-next-line:no-any
function emptyObject(obj: any) {
for (var x in obj) {
return false;
for (const key in obj) {
if (obj.hasOwnProperty(key)) {
return false;
}
}
return true;
}

interface LooseObject {
[key: string]: any
// just a map of string to string
interface Message {
[key: string]: string;
}

function findProps(node: ts.Node, tagName: string): LooseObject[] {
var res: LooseObject[] = [];
find(node);
function find(node: ts.Node): LooseObject[] {
if (!node) {
return undefined;
}
if (ts.isObjectLiteralExpression(node)) {
node.properties.forEach(p => {
var prop: LooseObject = {};
if (
ts.isPropertyAssignment(p) &&
ts.isObjectLiteralExpression(p.initializer) &&
p.initializer.properties
) {
p.initializer.properties.forEach(ip => {
if (ts.isIdentifier(ip.name)) {
let name = ip.name.text
if (ts.isPropertyAssignment(ip) && ts.isStringLiteral(ip.initializer)) {
prop[name] = ip.initializer.text;
}
}
});
res.push(prop);
type ElementName = "FormattedMessage";
type MethodName = "defineMessages" | "formatMessage";
type MessageExtracter = (obj: ts.ObjectLiteralExpression) => Message[];

function extractMessagesForDefineMessages(objLiteral: ts.ObjectLiteralExpression): Message[] {
const messages: Message[] = [];
objLiteral.properties.forEach((p) => {
const message: Message = {};
if (
ts.isPropertyAssignment(p) &&
ts.isObjectLiteralExpression(p.initializer) &&
p.initializer.properties
) {
p.initializer.properties.forEach((ip) => {
if (ts.isIdentifier(ip.name) || ts.isLiteralExpression(ip.name)) {
const name = ip.name.text;
if (ts.isPropertyAssignment(ip) && ts.isStringLiteral(ip.initializer)) {
message[name] = ip.initializer.text;
}
});

if (tagName === "formatMessage") {
var prop: LooseObject = {};
let name;

node.properties.forEach(p => {
if (ts.isPropertyAssignment(p) && ts.isStringLiteral(p.initializer)) {
name = (p.name as any).escapedText;
prop[name] = p.initializer.text;
}
});
}
});
messages.push(message);
}
});
return messages;
}

res.push(prop);
function extractMessagesForFormatMessage(objLiteral: ts.ObjectLiteralExpression): Message[] {
const message: Message = {};
objLiteral.properties.forEach((p) => {
if (
ts.isPropertyAssignment(p) &&
(ts.isIdentifier(p.name) || ts.isLiteralExpression(p.name)) &&
ts.isStringLiteral(p.initializer)
) {
message[p.name.text] = p.initializer.text;
}
});
return [message];
}

function extractMessagesForNode(node: ts.Node, extractMessages: MessageExtracter): Message[] {
const res: Message[] = [];
function find(n: ts.Node): Message[] {
if (ts.isObjectLiteralExpression(n)) {
res.push(...extractMessages(n));
} else {
return ts.forEachChild(n, find);
}
return ts.forEachChild(node, find);
}

find(node);
return res;
}

function forAllVarDecls(node: ts.Node, cb: (el: ts.VariableDeclaration) => void) {
function forAllVarDecls(node: ts.Node, cb: (decl: ts.VariableDeclaration) => void) {
if (ts.isVariableDeclaration(node)) {
cb(node)
cb(node);
} else {
ts.forEachChild(node, n => forAllVarDecls(n, cb))
ts.forEachChild(node, (n) => forAllVarDecls(n, cb));
}
}

function findFirstJsxOpeningLikeElementWithName(
function findJsxOpeningLikeElementsWithName(
node: ts.SourceFile,
tagName: string,
dm?: boolean
tagName: ElementName,
) {
var res: LooseObject[] = [];
find(node);

function find(node: ts.Node | ts.SourceFile): undefined {
if (!node) {
return undefined;
const messages: ts.JsxOpeningLikeElement[] = [];
function findJsxElement(n: ts.Node): undefined {
// Is this a JsxElement with an identifier name?
if (
ts.isJsxOpeningLikeElement(n) &&
ts.isIdentifier(n.tagName)
) {
// Does the tag name match what we're looking for?
const childTagName = n.tagName;
if (childTagName.text === tagName) {
// node is a JsxOpeningLikeElement
messages.push(n);
}
}
if (dm && ts.isSourceFile(node)) {
// getNamedDeclarations is not currently public
forAllVarDecls(node, (el: ts.Declaration) => {
if (isDefineMessages(el, tagName)) {
if (
ts.isCallExpression(el.initializer) &&
el.initializer.arguments.length
) {
var nodeProps = el.initializer.arguments[0];
var props = findProps(nodeProps, tagName);
// props is an array of LooseObject
res = res.concat(props);
}
}
})
} else {
// Is this a JsxElement with an identifier name?
return ts.forEachChild(n, findJsxElement);
}
findJsxElement(node);
return messages;
}

function findMethodCallsWithName(
sourceFile: ts.SourceFile,
methodName: MethodName,
extractMessages: MessageExtracter,
) {
let messages: Message[] = [];
// getNamedDeclarations is not currently public
forAllVarDecls(sourceFile, (decl: ts.Declaration) => {
if (isMethodCall(decl, methodName)) {
if (
ts.isJsxOpeningLikeElement(node) &&
ts.isIdentifier(node.tagName)
ts.isCallExpression(decl.initializer) &&
decl.initializer.arguments.length
) {
// Does the tag name match what we're looking for?
const childTagName = node.tagName;
if (childTagName.text === tagName) {
// node is a JsxOpeningLikeElement
res.push(node);
}
const nodeProps = decl.initializer.arguments[0];
const declMessages = extractMessagesForNode(nodeProps, extractMessages);
messages = messages.concat(declMessages);
}
}

return ts.forEachChild(node, find);
}

return res;
});
return messages;
}

/**
* Parse tsx files
*
* @export
* @param {string} contents
* @returns {array}
*/
// TODO perhaps we should expose the Message interface
Copy link
Collaborator Author

@seanf seanf Jan 14, 2018

Choose a reason for hiding this comment

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

@BANG88 How would you feel about exposing the Message interface as the return type for the (publicly exported) main method (well, an array of them to be exact)? (As opposed to an array of plain object {}.)

It shouldn't break any consumers, because I believe it's a narrower type, but I'm still very new to TypeScript, so I could be wrong about that. In practice the consumers seem to be plain JavaScript npm scripts (unless they use ts-node), so they probably don't care about types. Still, I like types as documentation.

Copy link
Owner

@BANG88 BANG88 Jan 14, 2018

Choose a reason for hiding this comment

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

I like the types too. If you prefer or think is useful for you please do it.

Copy link
Collaborator Author

@seanf seanf Jan 16, 2018

Choose a reason for hiding this comment

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

Thinking about it a bit more, I think it would be better if the interface looked more like this:

interface MessageDescriptor {
  id: string
  defaultMessage: string
  description?: string
  [key: string]: string; // for any other properties
}

Otherwise exposing it won't really add much value.

// tslint:disable-next-line:array-type
function main(contents: string): {}[] {
var sourceFile = ts.createSourceFile(
const sourceFile = ts.createSourceFile(
"file.ts",
contents,
ts.ScriptTarget.ES2015,
/*setParentNodes */ false,
ts.ScriptKind.TSX
ts.ScriptKind.TSX,
);

var elements = findFirstJsxOpeningLikeElementWithName(
const elements = findJsxOpeningLikeElementsWithName(
sourceFile,
"FormattedMessage"
"FormattedMessage",
);
var dm = findFirstJsxOpeningLikeElementWithName(
const dm = findMethodCallsWithName(
sourceFile,
"defineMessages",
true
extractMessagesForDefineMessages,
);
var fm = findFirstJsxOpeningLikeElementWithName(
const fm = findMethodCallsWithName(
sourceFile,
"formatMessage",
true
extractMessagesForFormatMessage,
);

var res = elements
.map(element => {
var msg: LooseObject = {};
// convert JsxOpeningLikeElements to Message maps
const jsxMessages = elements
.map((element) => {
const msg: Message = {};
element.attributes &&
element.attributes.properties.forEach((attr: LooseObject) => {
element.attributes.properties.forEach((attr: ts.JsxAttributeLike) => {
// found nothing
if (!attr.name || !attr.initializer) return;
msg[attr.name.text] =
attr.initializer.text || attr.initializer.expression.text;
// tslint:disable-next-line:no-any
const a = attr as any; // TODO find correct types to avoid "any"
if (!a.name || !a.initializer) { return; }
msg[a.name.text] =
a.initializer.text || a.initializer.expression.text;
});
return msg;
})
.filter(r => !emptyObject(r));
.filter((r) => !emptyObject(r));

return res.concat(dm).concat(fm);
return jsxMessages.concat(dm).concat(fm);
}

export default main;
Loading