Skip to content

Commit acea045

Browse files
johnplaistedlauraharker
johnplaisted
authored andcommitted
Remove code to reference types by path that doesn't really work. It uses dot (.) as a delimiter from path to type, meaning you can't use this if your module resolution isn't node (since you'd need to include the .js extension). It also only works within ES6 modules, which isn't great.
#3041 is open to implement something more robust. ------------- Created by MOE: https://github.com/google/moe MOE_MIGRATED_REVID=208718943
1 parent 1a8cca2 commit acea045

File tree

2 files changed

+23
-94
lines changed

2 files changed

+23
-94
lines changed

src/com/google/javascript/jscomp/Es6RewriteModules.java

Lines changed: 23 additions & 55 deletions
Original file line numberDiff line numberDiff line change
@@ -864,66 +864,34 @@ public void visit(NodeTraversal t, Node n, Node parent) {
864864
private void fixTypeNode(NodeTraversal t, Node typeNode) {
865865
if (typeNode.isString()) {
866866
String name = typeNode.getString();
867-
if (ModuleLoader.isPathIdentifier(name)) {
868-
int lastSlash = name.lastIndexOf('/');
869-
int endIndex = name.indexOf('.', lastSlash);
870-
String localTypeName = null;
871-
if (endIndex == -1) {
872-
endIndex = name.length();
867+
List<String> splitted = Splitter.on('.').limit(2).splitToList(name);
868+
String baseName = splitted.get(0);
869+
String rest = "";
870+
if (splitted.size() == 2) {
871+
rest = "." + splitted.get(1);
872+
}
873+
Var var = t.getScope().getVar(baseName);
874+
if (var != null && var.isGlobal()) {
875+
maybeSetNewName(t, typeNode, name, baseName + "$$" + suffix + rest);
876+
} else if (var == null && importMap.containsKey(baseName)) {
877+
ModuleOriginalNamePair pair = importMap.get(baseName);
878+
if (pair.originalName.isEmpty()) {
879+
maybeSetNewName(t, typeNode, name, pair.module + rest);
873880
} else {
874-
localTypeName = name.substring(endIndex);
875-
}
876-
877-
String moduleName = name.substring(0, endIndex);
878-
ModuleLoader.ModulePath path =
879-
t.getInput()
880-
.getPath()
881-
.resolveJsModule(
882-
moduleName,
883-
typeNode.getSourceFileName(),
884-
typeNode.getLineno(),
885-
typeNode.getCharno());
886-
887-
if (path == null) {
888-
path = t.getInput().getPath().resolveModuleAsPath(moduleName);
889-
}
890-
String globalModuleName = path.toModuleName();
891-
892-
maybeSetNewName(
893-
t,
894-
typeNode,
895-
name,
896-
localTypeName == null ? globalModuleName : globalModuleName + localTypeName);
897-
} else {
898-
List<String> splitted = Splitter.on('.').limit(2).splitToList(name);
899-
String baseName = splitted.get(0);
900-
String rest = "";
901-
if (splitted.size() == 2) {
902-
rest = "." + splitted.get(1);
881+
maybeSetNewName(t, typeNode, name, baseName + "$$" + pair.module + rest);
903882
}
904-
Var var = t.getScope().getVar(baseName);
905-
if (var != null && var.isGlobal()) {
906-
maybeSetNewName(t, typeNode, name, baseName + "$$" + suffix + rest);
907-
} else if (var == null && importMap.containsKey(baseName)) {
908-
ModuleOriginalNamePair pair = importMap.get(baseName);
909-
if (pair.originalName.isEmpty()) {
910-
maybeSetNewName(t, typeNode, name, pair.module + rest);
911-
} else {
912-
maybeSetNewName(t, typeNode, name, baseName + "$$" + pair.module + rest);
913-
}
914883

915-
if (preprocessorSymbolTable != null) {
916-
// Jsdoc type node is a single STRING node that spans the whole type. For example
917-
// STRING node "bar.Foo". ES6 import rewrite replaces only "module"
918-
// part of the type: "bar.Foo" => "module$full$path$bar$Foo". We have to record
919-
// "bar" as alias.
920-
Node onlyBaseName = Node.newString(baseName).useSourceInfoFrom(typeNode);
921-
onlyBaseName.setLength(baseName.length());
922-
maybeAddAliasToSymbolTable(onlyBaseName, t.getSourceName());
923-
}
884+
if (preprocessorSymbolTable != null) {
885+
// Jsdoc type node is a single STRING node that spans the whole type. For example
886+
// STRING node "bar.Foo". ES6 import rewrite replaces only "module"
887+
// part of the type: "bar.Foo" => "module$full$path$bar$Foo". We have to record
888+
// "bar" as alias.
889+
Node onlyBaseName = Node.newString(baseName).useSourceInfoFrom(typeNode);
890+
onlyBaseName.setLength(baseName.length());
891+
maybeAddAliasToSymbolTable(onlyBaseName, t.getSourceName());
924892
}
925-
typeNode.setOriginalName(name);
926893
}
894+
typeNode.setOriginalName(name);
927895
}
928896

929897
for (Node child = typeNode.getFirstChild(); child != null; child = child.getNext()) {

test/com/google/javascript/jscomp/Es6RewriteModulesTest.java

Lines changed: 0 additions & 39 deletions
Original file line numberDiff line numberDiff line change
@@ -653,45 +653,6 @@ public void testFixTypeNode() {
653653
"/** @const */ module$testcode.Child = Child$$module$testcode;"));
654654
}
655655

656-
public void testReferenceToTypeFromOtherModule() {
657-
setModuleResolutionMode(ModuleLoader.ResolutionMode.NODE);
658-
testModules(
659-
lines(
660-
"export class Foo {", " /** @param {./other.Baz} baz */", " useBaz(baz) {}", "}"),
661-
lines(
662-
"class Foo$$module$testcode {",
663-
" /** @param {module$other.Baz} baz */",
664-
" useBaz(baz) {}",
665-
"}",
666-
"/** @const */ var module$testcode = {};",
667-
"/** @const */ module$testcode.Foo = Foo$$module$testcode;"));
668-
669-
testModules(
670-
lines(
671-
"export class Foo {", " /** @param {/other.Baz} baz */", " useBaz(baz) {}", "}"),
672-
lines(
673-
"class Foo$$module$testcode {",
674-
" /** @param {module$other.Baz} baz */",
675-
" useBaz(baz) {}",
676-
"}",
677-
"/** @const */ var module$testcode = {};",
678-
"/** @const */ module$testcode.Foo = Foo$$module$testcode;"));
679-
680-
testModules(
681-
lines(
682-
"import {Parent} from './other.js';",
683-
"class Child extends Parent {",
684-
" /** @param {./other.Parent} parent */",
685-
" useParent(parent) {}",
686-
"}"),
687-
lines(
688-
"class Child$$module$testcode extends module$other.Parent {",
689-
" /** @param {module$other.Parent} parent */",
690-
" useParent(parent) {}",
691-
"}",
692-
"/** @const */ var module$testcode = {};"));
693-
}
694-
695656
public void testRenameTypedef() {
696657
testModules(
697658
lines(

0 commit comments

Comments
 (0)