Skip to content

Commit ec31e72

Browse files
johnplaistedbrad4d
johnplaisted
authored and
brad4d
committed
Fix ES modules referencing CJS modules and add unit tests for this interop.
CJS modules should act like an ESM that only has a default export. ------------- Created by MOE: https://github.com/google/moe MOE_MIGRATED_REVID=252830083
1 parent 3392845 commit ec31e72

File tree

4 files changed

+180
-9
lines changed

4 files changed

+180
-9
lines changed

‎src/com/google/javascript/jscomp/modules/EsModuleProcessor.java

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -404,7 +404,8 @@ private ResolveExportResult resolveImportImpl(
404404
} else {
405405
boolean importStar = i.importName().equals("*");
406406
if (importStar
407-
|| (i.importName().equals(Export.DEFAULT) && !requested.metadata().isEs6Module())) {
407+
|| (i.importName().equals(Export.DEFAULT)
408+
&& (requested.metadata().isGoogProvide() || requested.metadata().isGoogModule()))) {
408409
if (!GoogEsImports.isGoogImportSpecifier(i.moduleRequest())
409410
&& (requested.metadata().isGoogModule() || requested.metadata().isGoogProvide())) {
410411
compiler.report(

‎src/com/google/javascript/jscomp/modules/NonEsModuleProcessor.java

Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -65,6 +65,13 @@ public ResolveExportResult resolveExport(
6565
if (moduleSpecifier != null && GoogEsImports.isGoogImportSpecifier(moduleSpecifier)) {
6666
namespace = GoogEsImports.getClosureIdFromGoogImportSpecifier(moduleSpecifier);
6767
}
68+
if (metadata.isCommonJs()) {
69+
// Currently we don't scan require()s, so this only gets hit if an ES module imports a CJS
70+
// module. If so then this module should only have a default export.
71+
if (!exportName.equals("default")) {
72+
return ResolveExportResult.NOT_FOUND;
73+
}
74+
}
6875
return ResolveExportResult.of(
6976
Binding.from(
7077
Export.builder()

‎test/com/google/javascript/jscomp/CommonJSIntegrationTest.java

Lines changed: 164 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -16,8 +16,13 @@
1616

1717
package com.google.javascript.jscomp;
1818

19+
import static com.google.javascript.jscomp.CheckGlobalNames.UNDEFINED_NAME_WARNING;
20+
import static com.google.javascript.jscomp.TypeCheck.INEXISTENT_PROPERTY;
21+
1922
import com.google.javascript.jscomp.CompilerOptions.LanguageMode;
2023
import com.google.javascript.jscomp.deps.ModuleLoader;
24+
import com.google.javascript.jscomp.modules.ModuleMapCreator;
25+
import org.junit.Ignore;
2126
import org.junit.Test;
2227
import org.junit.runner.RunWith;
2328
import org.junit.runners.JUnit4;
@@ -262,16 +267,173 @@ public void testCrossModuleSubclass6() {
262267
});
263268
}
264269

270+
@Test
271+
@Ignore // TODO(ChadKillingsworth)
272+
public void testCommonJSImportsESModule() {
273+
test(
274+
createCompilerOptions(),
275+
new String[] {
276+
lines(
277+
"/** @constructor */ export default function Hello() {};", //
278+
"export const foo = 1;"),
279+
lines(
280+
"var i0 = require('./i0');",
281+
"var util = {inherits: function (x, y){}};",
282+
"/**",
283+
" * @constructor",
284+
" * @extends {./i0.default}",
285+
" */",
286+
"function SubHello() { i0.default.call(this); }",
287+
"util.inherits(SubHello, i0.default);",
288+
"const {foo} = i0;")
289+
},
290+
new String[] {
291+
lines(
292+
"function Hello$$module$i0() {}",
293+
"var foo$$module$i0 = 1;",
294+
"var module$i0 = {};",
295+
"module$i0.default = Hello$$module$i0;",
296+
"module$i0.foo = foo$$module$i0;"),
297+
lines(
298+
"var i0 = module$i0;",
299+
"var util = {inherits:function(x,y){}};",
300+
"function SubHello(){ module$i0.default.call(this); }",
301+
"util.inherits(SubHello, module$i0.default);",
302+
"var $jscomp$destructuring$var0 = module$i0;",
303+
"var foo = $jscomp$destructuring$var0.foo;")
304+
});
305+
}
306+
307+
@Test
308+
public void testESModuleDefaultImportsCommonJS_inheritance() {
309+
test(
310+
createCompilerOptions(),
311+
new String[] {
312+
"/** @constructor */ function Hello() {} module.exports = Hello;",
313+
lines(
314+
"import Hello from './i0';",
315+
"var util = {inherits: function (x, y){}};",
316+
"/**",
317+
" * @constructor",
318+
" * @extends {Hello}",
319+
" */",
320+
"function SubHello() { Hello.call(this); }",
321+
"util.inherits(SubHello, Hello);")
322+
},
323+
new String[] {
324+
lines(
325+
"/** @const */ var module$i0 = {};",
326+
"/** @const */ module$i0.default = /** @constructor */ function (){};"),
327+
lines(
328+
"var util$$module$i1 = {inherits:function(x,y){}};",
329+
"/**",
330+
" * @constructor",
331+
" * @extends {module$i0.default}",
332+
" */",
333+
"function SubHello$$module$i1(){ module$i0.default.call(this); }",
334+
"util$$module$i1.inherits(SubHello$$module$i1, module$i0.default);",
335+
"var module$i1 = {};")
336+
});
337+
}
338+
339+
@Test
340+
public void testESModuleStarImportsCommonJS() {
341+
test(
342+
createCompilerOptions(),
343+
new String[] {
344+
"/** @constructor */ function Hello() {} module.exports = Hello;",
345+
lines(
346+
"import * as i0 from './i0';",
347+
"var util = {inherits: function (x, y){}};",
348+
"/**",
349+
" * @constructor",
350+
" * @extends {i0.default}",
351+
" */",
352+
"function SubHello() { i0.default.call(this); }",
353+
"util.inherits(SubHello, i0.default);")
354+
},
355+
new String[] {
356+
lines(
357+
"/** @const */ var module$i0 = {};",
358+
"/** @const */ module$i0.default = /** @constructor */ function (){};"),
359+
lines(
360+
"var util$$module$i1 = {inherits:function(x,y){}};",
361+
"function SubHello$$module$i1(){ module$i0.default.call(this); }",
362+
"util$$module$i1.inherits(SubHello$$module$i1, module$i0.default);",
363+
"var module$i1 = {};")
364+
});
365+
}
366+
367+
@Test
368+
public void testESModuleDefaultImportsCommonJS_variableReference() {
369+
test(
370+
createCompilerOptions(),
371+
new String[] {
372+
"module.exports = {foo: 1, bar: function() { return 'bar'; }};",
373+
lines(
374+
"import i0 from './i0';", //
375+
"const {foo} = i0;",
376+
"const b = i0.bar();")
377+
},
378+
new String[] {
379+
lines(
380+
"/** @const */ var module$i0 = {/** @const */ default: {}};",
381+
"module$i0.default.foo = 1;",
382+
"module$i0.default.bar = function() { return 'bar'; };"),
383+
lines(
384+
"var $jscomp$destructuring$var0 = module$i0.default;",
385+
"var foo$$module$i1 = $jscomp$destructuring$var0.foo;",
386+
"var b$$module$i1 = module$i0.default.bar();",
387+
"var module$i1 = {};")
388+
});
389+
}
390+
391+
@Test
392+
public void testEsModuleImportNonDefaultFromCJS() {
393+
test(
394+
createCompilerOptions(),
395+
new String[] {
396+
"module.exports = {foo: 1, bar: function() { return 'bar'; }};",
397+
"import {foo} from './i0';"
398+
},
399+
ModuleMapCreator.DOES_NOT_HAVE_EXPORT);
400+
}
401+
402+
@Test
403+
public void testEsModuleImportStarNonDefaultFromCJS() {
404+
test(
405+
createCompilerOptions(),
406+
new String[] {
407+
"module.exports = {foo: 1, bar: function() { return 'bar'; }};",
408+
"import * as m from './i0'; m.bar();"
409+
},
410+
new String[] {
411+
lines(
412+
"/** @const */ var module$i0 = {/** @const */ default:{}};",
413+
"module$i0.default.foo = 1;",
414+
"module$i0.default.bar = function() {",
415+
" return \"bar\";",
416+
"};"),
417+
lines(
418+
"module$i0.bar();", //
419+
"/** @const */ var module$i1 = {};")
420+
},
421+
new DiagnosticType[] {UNDEFINED_NAME_WARNING, INEXISTENT_PROPERTY});
422+
}
423+
265424
@Override
266425
protected CompilerOptions createCompilerOptions() {
267426
CompilerOptions options = new CompilerOptions();
268-
options.setLanguageIn(LanguageMode.ECMASCRIPT5);
269-
options.setLanguageOut(LanguageMode.ECMASCRIPT3);
427+
options.setLanguageIn(LanguageMode.ECMASCRIPT_2015);
428+
options.setLanguageOut(LanguageMode.ECMASCRIPT5);
270429
options.setCodingConvention(new GoogleCodingConvention());
271430
WarningLevel.VERBOSE.setOptionsForWarningLevel(options);
272431
options.setProcessCommonJSModules(true);
273432
options.setClosurePass(true);
274433
options.setModuleResolutionMode(ModuleLoader.ResolutionMode.NODE);
434+
// For debugging failures
435+
options.setPrettyPrint(true);
436+
options.preserveTypeAnnotations = true;
275437
return options;
276438
}
277439
}

‎test/com/google/javascript/jscomp/IntegrationTestCase.java

Lines changed: 7 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -355,6 +355,13 @@ protected void test(CompilerOptions options,
355355
protected void test(CompilerOptions options,
356356
String[] original, String[] compiled) {
357357
Compiler compiler = compile(options, original);
358+
359+
Node root = compiler.getJsRoot();
360+
if (compiled != null) {
361+
Node expectedRoot = parseExpectedCode(compiled, options, normalizeResults);
362+
assertNode(root).usingSerializer(compiler::toSource).isEqualTo(expectedRoot);
363+
}
364+
358365
assertWithMessage(
359366
"Expected no warnings or errors\n"
360367
+ "Errors: \n"
@@ -364,12 +371,6 @@ protected void test(CompilerOptions options,
364371
+ Joiner.on("\n").join(compiler.getWarnings()))
365372
.that(compiler.getErrors().size() + compiler.getWarnings().size())
366373
.isEqualTo(0);
367-
368-
Node root = compiler.getJsRoot();
369-
if (compiled != null) {
370-
Node expectedRoot = parseExpectedCode(compiled, options, normalizeResults);
371-
assertNode(root).usingSerializer(compiler::toSource).isEqualTo(expectedRoot);
372-
}
373374
}
374375

375376
/**

0 commit comments

Comments
 (0)