Skip to content

Commit

Permalink
Handle ES6 module exports for component types.
Browse files Browse the repository at this point in the history
Rewrites the export into a declaration + export (so that @typedef
continues to work) and adds exports for the generated types.

CC=@arv,@rafaelweinstein
  • Loading branch information
mihaip committed Jan 31, 2019
1 parent 86159d2 commit 0fbae2c
Show file tree
Hide file tree
Showing 2 changed files with 75 additions and 3 deletions.
53 changes: 51 additions & 2 deletions src/info/persistent/react/jscomp/ReactCompilerPass.java
Expand Up @@ -74,6 +74,9 @@ public class ReactCompilerPass implements NodeTraversal.Callback,
DiagnosticType.error(
"REACT_PURE_RENDER_MIXIN_SHOULD_COMPONENT_UPDATE_OVERRIDE",
"{0} uses React.addons.PureRenderMixin, it should not define shouldComponentUpdate.");
static final DiagnosticType UNEXPECTED_EXPORT_SYNTAX = DiagnosticType.error(
"REACT_UNEXPECTED_EXPORT_SYNTAX",
"Unexpected export syntax.");

public static final DiagnosticGroup MALFORMED_MIXINS = new DiagnosticGroup(
MIXINS_UNEXPECTED_TYPE, MIXIN_EXPECTED_NAME, MIXIN_UNKNOWN);
Expand Down Expand Up @@ -436,6 +439,36 @@ private void visitReactCreateType(
}
String interfaceTypeName = generateInterfaceTypeName(typeName);

// Check to see if the type is has an ES6 module export. We assume this is

This comment has been minimized.

Copy link
@arv

arv Jan 31, 2019

Collaborator

nit: type is has

This comment has been minimized.

Copy link
@mihaip

mihaip Feb 4, 2019

Author Owner

Thanks, fixed with d7fb9ee

// of the form `export const Comp = React.createClass(...)`. If it is, then
// we transform it into `const Comp = React.createClass(...); export {Comp};`.
// That way it's more similar to non-module uses (as far as where we can
// add additional nodes to the AST) and the @typedef that we use for the
// declaration does not prevent it from getting exported (Es6RewriteModules
// does not export the values of typedefs).
// Additionally, if the component is exported, then we also need to export
// types that we generate from it (CompInterface, CompElement).
boolean addModuleExports = false;
for (Node ancestor : callNode.getAncestors()) {
if (!ancestor.isExport()) {
continue;
}
addModuleExports = true;
Node exportNode = ancestor;
if (!callParentNode.isName() ||
!NodeUtil.isNameDeclaration(callParentNode.getParent())) {
compiler.report(
JSError.make(callParentNode, UNEXPECTED_EXPORT_SYNTAX));
return;
}
Node nameNode = callParentNode;
Node declarationNode = nameNode.getParent();
declarationNode.detachFromParent();
exportNode.replaceWith(declarationNode);
addModuleExport(typeName, declarationNode);
break;
}

// For compomnents tagged with @export don't rename their props or public
// methods.
JSDocInfo jsDocInfo = NodeUtil.getBestJSDocInfo(callNode);
Expand Down Expand Up @@ -577,8 +610,8 @@ private void visitReactCreateType(
jsDocBuilder.recordTypedef(new JSTypeExpression(
createReactElementTypeExpressionNode(typeName),
callNode.getSourceFileName()));
Node elementTypedefNode = NodeUtil.newQName(
compiler, typeName + "Element");
String elementTypeName = typeName + "Element";
Node elementTypedefNode = NodeUtil.newQName(compiler, elementTypeName);
if (elementTypedefNode.isName()) {
elementTypedefNode = IR.var(elementTypedefNode);
}
Expand All @@ -590,6 +623,9 @@ private void visitReactCreateType(
Node typesInsertionPoint = callParentNode.getParent();
typesInsertionPoint.getParent().addChildAfter(
elementTypedefNode, typesInsertionPoint);
if (addModuleExports) {
addModuleExport(elementTypeName, elementTypedefNode);
}

// Generate statics property JSDocs, so that the compiler knows about them.
if (createFuncName.equals("React.createClass")) {
Expand Down Expand Up @@ -645,6 +681,9 @@ private void visitReactCreateType(
interfaceTypeFunctionNode.setJSDocInfo(jsDocBuilder.build());
typesInsertionPoint.getParent().addChildBefore(
interfaceTypeNode, typesInsertionPoint);
if (addModuleExports) {
addModuleExport(interfaceTypeName, interfaceTypeNode);
}
typesInsertionPoint.getParent().addChildAfter(
NodeUtil.newQNameDeclaration(
compiler,
Expand Down Expand Up @@ -1175,4 +1214,14 @@ private String generateInterfaceTypeName(String typeName) {

return typeNamePrefix + "Interface";
}

private static void addModuleExport(String name, Node insertionPoint) {
Node exportSpecNode = new Node(Token.EXPORT_SPEC).srcref(insertionPoint);
Node nameNode = IR.name(name).srcref(insertionPoint);
exportSpecNode.addChildToBack(nameNode);
exportSpecNode.addChildToBack(nameNode.cloneNode());
Node exportSpecsNode = new Node(Token.EXPORT_SPECS, exportSpecNode).srcref(insertionPoint);
Node exportNode = IR.export(exportSpecsNode).srcref(insertionPoint);
insertionPoint.getParent().addChildAfter(exportNode, insertionPoint);
}
}
25 changes: 24 additions & 1 deletion test/info/persistent/react/jscomp/ReactCompilerPassTest.java
Expand Up @@ -55,7 +55,7 @@ public class ReactCompilerPassTest {
"}" +
"});" +
"ReactDOM.render(React.createElement(Comp), document.body);",
// createClass and other React methods should get renamed.
// createClass and other React methods should not get renamed.
"ReactDOM.render(React.createElement(React.createClass({" +
"render:function(){" +
"return React.createElement(" +
Expand All @@ -64,6 +64,29 @@ public class ReactCompilerPassTest {
"})),document.body);");
}

@Test public void testEs6Modules() {
test(
"export const Comp = React.createClass({" +
"render: function() {" +
"return React.createElement(" +
"\"div\", null, React.createElement(\"span\", null, \"child\"));" +
"}" +
"});" +
FILE_SEPARATOR +
// Test that we can use the component and associated types from another
// module (i.e. that exports are generated for them).
"import * as file1 from './file1.js';\n" +
"const /** file1.CompElement */ compElement = React.createElement(file1.Comp);\n" +
"const /** file1.CompInterface */ compInstance = ReactDOM.render(compElement, document.body);",
"var $compElement$$module$src$file2$$=React.createElement(React.createClass({" +
"render:function(){" +
"return React.createElement(" +
"\"div\",null,React.createElement(\"span\",null,\"child\"))" +
"}" +
"}));" +
"ReactDOM.render($compElement$$module$src$file2$$,document.body);");
}

@Test public void testInstanceMethods() {
test(
"var Comp = React.createClass({" +
Expand Down

1 comment on commit 0fbae2c

@arv
Copy link
Collaborator

@arv arv commented on 0fbae2c Jan 31, 2019

Choose a reason for hiding this comment

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

LGTM

Please sign in to comment.