Replace all slashes with periods in the module name instead of just the first (#154)

* Replace all slashes with periods in the module name instead of just the first

* Standardize the pathToModule function across tests and the CLI

Author: jjudd

src/cli_support.ts

@@ -0,0 +1,9 @@
+import * as path from 'path';
+
+// Postprocess generated JS.
+export function pathToModuleName(context: string, fileName: string): string {
+  if (fileName[0] === '.') {
+    fileName = path.join(path.dirname(context), fileName);
+  }
+  return fileName.replace(/\.js$/, '').replace(/\//g, '.');
+}

src/main.ts

@@ -6,6 +6,7 @@ import * as minimist from 'minimist';
 import * as path from 'path';
 import * as ts from 'typescript';
 
+import * as cli_support from './cli_support';
 import * as tsickle from './tsickle';
 
 /**
@@ -208,14 +209,9 @@ function toClosureJS(options: ts.CompilerOptions, fileNames: string[]):
     return {errors: diagnostics};
   }
 
-  // Postprocess generated JS.
-  function pathToModuleName(context: string, fileName: string): string {
-    // TODO(evanm): something more sophisticated here?
-    return fileName.replace('/', '.');
-  }
   for (let fileName of Object.keys(jsFiles)) {
-    let {output} =
-        tsickle.convertCommonJsToGoogModule(fileName, jsFiles[fileName], pathToModuleName);
+    let {output} = tsickle.convertCommonJsToGoogModule(
+        fileName, jsFiles[fileName], cli_support.pathToModuleName);
     jsFiles[fileName] = output;
   }
 

test/test_support.ts

@@ -3,9 +3,9 @@ import * as glob from 'glob';
 import * as path from 'path';
 import * as ts from 'typescript';
 
+import * as cli_support from '../src/cli_support';
 import * as tsickle from '../src/tsickle';
 
-
 /** The TypeScript compiler options used by the test suite. */
 const compilerOptions: ts.CompilerOptions = {
   target: ts.ScriptTarget.ES6,
@@ -56,19 +56,12 @@ export function emit(program: ts.Program): {[fileName: string]: string} {
   let transformed: {[fileName: string]: string} = {};
   let emitRes = program.emit(undefined, (fileName: string, data: string) => {
     transformed[fileName] =
-        tsickle.convertCommonJsToGoogModule(fileName, data, pathToModuleName).output;
+        tsickle.convertCommonJsToGoogModule(fileName, data, cli_support.pathToModuleName).output;
   });
   if (emitRes.diagnostics.length) {
     throw new Error(tsickle.formatDiagnostics(emitRes.diagnostics));
   }
   return transformed;
-
-  function pathToModuleName(context: string, fileName: string): string {
-    if (fileName[0] === '.') {
-      fileName = path.join(path.dirname(context), fileName);
-    }
-    return fileName.replace(/\.js$/, '').replace(/\//g, '.');
-  }
 }
 
 export class GoldenFileTest {

test/tsickle_test.ts

@@ -3,6 +3,7 @@ import * as fs from 'fs';
 import * as path from 'path';
 import * as ts from 'typescript';
 
+import * as cli_support from '../src/cli_support';
 import * as tsickle from '../src/tsickle';
 
 import * as test_support from './test_support';
@@ -164,102 +165,96 @@ describe('getJSDocAnnotation', () => {
 });
 
 describe('convertCommonJsToGoogModule', () => {
-  function pathToModuleName(context: string, fileName: string) {
-    if (fileName[0] === '.') {
-      fileName = path.join(path.dirname(context), fileName);
-    }
-    return fileName.replace(/\//g, '$').replace(/_/g, '__');
-  }
-
   function expectCommonJs(fileName: string, content: string) {
-    fileName = fileName.substring(0, fileName.lastIndexOf('.'));
-    return expect(tsickle.convertCommonJsToGoogModule(fileName, content, pathToModuleName).output);
+    return expect(
+        tsickle.convertCommonJsToGoogModule(fileName, content, cli_support.pathToModuleName)
+            .output);
   }
 
   it('adds a goog.module call', () => {
     // NB: no line break added below.
     expectCommonJs('a.js', `console.log('hello');`)
-        .to.equal(`goog.module('a');var module = module || {id: 'a'};console.log('hello');`);
+        .to.equal(`goog.module('a');var module = module || {id: 'a.js'};console.log('hello');`);
   });
 
   it('adds a goog.module call to empty files', () => {
-    expectCommonJs('a.js', ``).to.equal(`goog.module('a');var module = module || {id: 'a'};`);
+    expectCommonJs('a.js', ``).to.equal(`goog.module('a');var module = module || {id: 'a.js'};`);
   });
 
   it('adds a goog.module call to empty-looking files', () => {
     expectCommonJs('a.js', `// empty`)
-        .to.equal(`goog.module('a');var module = module || {id: 'a'};// empty`);
+        .to.equal(`goog.module('a');var module = module || {id: 'a.js'};// empty`);
   });
 
   it('strips use strict directives', () => {
     // NB: no line break added below.
     expectCommonJs('a.js', `"use strict";
 console.log('hello');`)
-        .to.equal(`goog.module('a');var module = module || {id: 'a'};
+        .to.equal(`goog.module('a');var module = module || {id: 'a.js'};
 console.log('hello');`);
   });
 
   it('converts require calls', () => {
     expectCommonJs('a.js', `var r = require('req/mod');`)
         .to.equal(
-            `goog.module('a');var module = module || {id: 'a'};var r = goog.require('req$mod');`);
+            `goog.module('a');var module = module || {id: 'a.js'};var r = goog.require('req.mod');`);
   });
 
   it('converts require calls without assignments on first line', () => {
     expectCommonJs('a.js', `require('req/mod');`)
         .to.equal(
-            `goog.module('a');var module = module || {id: 'a'};var unused_0_ = goog.require('req$mod');`);
+            `goog.module('a');var module = module || {id: 'a.js'};var unused_0_ = goog.require('req.mod');`);
   });
 
   it('converts require calls without assignments on a new line', () => {
     expectCommonJs('a.js', `
 require('req/mod');
 require('other');`)
-        .to.equal(`goog.module('a');var module = module || {id: 'a'};
-var unused_0_ = goog.require('req$mod');
+        .to.equal(`goog.module('a');var module = module || {id: 'a.js'};
+var unused_0_ = goog.require('req.mod');
 var unused_1_ = goog.require('other');`);
   });
 
   it('converts require calls without assignments after comments', () => {
     expectCommonJs('a.js', `
 // Comment
 require('req/mod');`)
-        .to.equal(`goog.module('a');var module = module || {id: 'a'};
+        .to.equal(`goog.module('a');var module = module || {id: 'a.js'};
 // Comment
-var unused_0_ = goog.require('req$mod');`);
+var unused_0_ = goog.require('req.mod');`);
   });
 
   it('converts const require calls', () => {
     expectCommonJs('a.js', `const r = require('req/mod');`)
         .to.equal(
-            `goog.module('a');var module = module || {id: 'a'};var r = goog.require('req$mod');`);
+            `goog.module('a');var module = module || {id: 'a.js'};var r = goog.require('req.mod');`);
   });
 
   it('converts export * statements', () => {
     expectCommonJs('a.js', `__export(require('req/mod'));`)
         .to.equal(
-            `goog.module('a');var module = module || {id: 'a'};__export(goog.require('req$mod'));`);
+            `goog.module('a');var module = module || {id: 'a.js'};__export(goog.require('req.mod'));`);
   });
 
   it('resolves relative module URIs', () => {
     // See below for more fine-grained unit tests.
     expectCommonJs('a/b.js', `var r = require('./req/mod');`)
         .to.equal(
-            `goog.module('a$b');var module = module || {id: 'a/b'};var r = goog.require('a$req$mod');`);
+            `goog.module('a.b');var module = module || {id: 'a/b.js'};var r = goog.require('a.req.mod');`);
   });
 
   it('avoids mangling module names in goog: imports', () => {
     expectCommonJs('a/b.js', `
 var goog_use_Foo_1 = require('goog:foo_bar.baz');`)
-        .to.equal(`goog.module('a$b');var module = module || {id: 'a/b'};
+        .to.equal(`goog.module('a.b');var module = module || {id: 'a/b.js'};
 var goog_use_Foo_1 = goog.require('foo_bar.baz');`);
   });
 
   it('resolves default goog: module imports', () => {
     expectCommonJs('a/b.js', `
 var goog_use_Foo_1 = require('goog:use.Foo');
 console.log(goog_use_Foo_1.default);`)
-        .to.equal(`goog.module('a$b');var module = module || {id: 'a/b'};
+        .to.equal(`goog.module('a.b');var module = module || {id: 'a/b.js'};
 var goog_use_Foo_1 = goog.require('use.Foo');
 console.log(goog_use_Foo_1        );`);
     // NB: the whitespace above matches the .default part, so that
@@ -271,7 +266,7 @@ console.log(goog_use_Foo_1        );`);
     expectCommonJs('a/b.js', `
 console.log(this.default);
 console.log(foo.bar.default);`)
-        .to.equal(`goog.module('a$b');var module = module || {id: 'a/b'};
+        .to.equal(`goog.module('a.b');var module = module || {id: 'a/b.js'};
 console.log(this.default);
 console.log(foo.bar.default);`);
   });
@@ -282,7 +277,7 @@ console.log(foo.bar.default);`);
 */
 "use strict";
 var foo = bar;
-`).to.equal(`goog.module('a$b');var module = module || {id: 'a/b'};/**
+`).to.equal(`goog.module('a.b');var module = module || {id: 'a/b.js'};/**
 * docstring here
 */
 
@@ -294,7 +289,7 @@ var foo = bar;
     expectCommonJs('a/b.js', `var foo_1 = require('goog:foo');
 var foo_2 = require('goog:foo');
 foo_1.A, foo_2.B, foo_2.default, foo_3.default;
-`).to.equal(`goog.module('a$b');var module = module || {id: 'a/b'};var foo_1 = goog.require('foo');
+`).to.equal(`goog.module('a.b');var module = module || {id: 'a/b.js'};var foo_1 = goog.require('foo');
 var foo_2 = foo_1;
 foo_1.A, foo_2.B, foo_2        , foo_3.default;
 `);
@@ -309,14 +304,14 @@ var es6RelativeRequire = require('./relative');
 var es6NonRelativeRequire = require('non/relative');
 __export(require('./export-star');
 `,
-        pathToModuleName);
+        cli_support.pathToModuleName);
 
     return expect(referencedModules).to.deep.equal([
-      'foo$bare-require',
+      'foo.bare-require',
       'foo.bar',
-      'a$relative',
-      'non$relative',
-      'a$export-star',
+      'a.relative',
+      'non.relative',
+      'a.export-star',
     ]);
   });
 });