fix: make sure generated protos.js have unique root name (#774)

* fix: make sure generated protos.js have unique root name

* fix: lint

samples/system-test/test.pagination.js

@@ -30,6 +30,6 @@ const cwd = path.join(__dirname, '..');
 describe('Pagination', () => {
   it('should run pagination sample', async () => {
     const stdout = execSync('node pagination.js', {cwd});
-    assert.match(stdout, new RegExp(`'cat', 'dog', 'snake', 'turtle', 'wolf'`));
+    assert.match(stdout, new RegExp("'cat', 'dog', 'snake', 'turtle', 'wolf'"));
   });
 });

samples/system-test/test.quickstart.js

@@ -30,8 +30,8 @@ const cwd = path.join(__dirname, '..');
 describe('Quickstart', () => {
   it('should run quickstart sample', async () => {
     const stdout = execSync('node quickstart.js', {cwd});
-    assert.match(stdout, new RegExp(`This call failed`));
-    assert.match(stdout, new RegExp(`This call succeeded`));
-    assert.match(stdout, new RegExp(`response: 'ok'`));
+    assert.match(stdout, new RegExp('This call failed'));
+    assert.match(stdout, new RegExp('This call succeeded'));
+    assert.match(stdout, new RegExp("response: 'ok'"));
   });
 });

test/fixtures/package.json

@@ -0,0 +1,5 @@
+{
+  "_why?": "This is a fake package.json for compileProtos test.",
+  "name": "@org/fake-package",
+  "version": "42.0.7-prealpha84"
+}

test/system-test/showcase-server.ts

@@ -66,7 +66,7 @@ export class ShowcaseServer {
 
   stop() {
     if (!this.server) {
-      throw new Error(`Cannot kill the server, it's not started.`);
+      throw new Error("Cannot kill the server, it's not started.");
     }
     this.server.kill();
   }

test/unit/compileProtos.ts

@@ -70,6 +70,13 @@ describe('compileProtos tool', () => {
       js.toString().includes('http://www.apache.org/licenses/LICENSE-2.0')
     );
 
+    // check that it uses proper root object; it's taken from fixtures/package.json
+    assert(
+      js
+        .toString()
+        .includes('$protobuf.roots._org_fake_package_42_0_7_prealpha84')
+    );
+
     const ts = await readFile(expectedTSResultFile);
     assert(ts.toString().includes('TestMessage'));
     assert(ts.toString().includes('LibraryService'));
@@ -92,9 +99,6 @@ describe('compileProtos tool', () => {
       ),
     ]);
     assert(fs.existsSync(expectedJsonResultFile));
-
-    const json = await readFile(expectedJsonResultFile);
-    assert.strictEqual(json.toString(), '{}');
   });
 
   it('fixes types in the TS file', async () => {
@@ -141,4 +145,26 @@ describe('compileProtos tool', () => {
         )
     );
   });
+
+  it('proposes the name for protobuf root', async () => {
+    const rootName = await compileProtos.generateRootName([
+      path.join(__dirname, '..', '..', 'test', 'fixtures', 'dts-update'),
+    ]);
+    assert.strictEqual(rootName, '_org_fake_package_42_0_7_prealpha84_protos');
+  });
+
+  it('falls back to the default name for protobuf root if unable to guess', async () => {
+    const rootName = await compileProtos.generateRootName([
+      path.join(
+        __dirname,
+        '..',
+        '..',
+        'test',
+        'fixtures',
+        'protoLists',
+        'empty'
+      ),
+    ]);
+    assert.strictEqual(rootName, 'default');
+  });
 });

tools/compileProtos.ts

@@ -198,9 +198,13 @@ async function buildListOfProtos(protoJsonFiles: string[]): Promise<string[]> {
  * `./protos/protos.json`. No support for changing output filename for now
  * (but it's a TODO!)
  *
+ * @param {string} rootName Name of the root object for pbjs static module (-r option)
  * @param {string[]} protos List of proto files to compile.
  */
-async function compileProtos(protos: string[]): Promise<void> {
+async function compileProtos(
+  rootName: string,
+  protos: string[]
+): Promise<void> {
   // generate protos.json file from proto list
   const jsonOutput = path.join('protos', 'protos.json');
   if (protos.length === 0) {
@@ -224,6 +228,8 @@ async function compileProtos(protos: string[]): Promise<void> {
   // generate protos/protos.js from protos.json
   const jsOutput = path.join('protos', 'protos.js');
   const pbjsArgs4js = [
+    '-r',
+    rootName,
     '--target',
     'static-module',
     '-p',
@@ -251,6 +257,34 @@ async function compileProtos(protos: string[]): Promise<void> {
   await writeFile(tsOutput, tsResult);
 }
 
+/**
+ *
+ * @param directories List of directories to process. Normally, just the
+ * `./src` folder of the given client library.
+ * @return {Promise<string>} Resolves to a unique name for protobuf root to use in the JS static module, or 'default'.
+ */
+export async function generateRootName(directories: string[]): Promise<string> {
+  // We need to provide `-r root` option to `pbjs -t static-module`, otherwise
+  // we'll have big problems if two different libraries are used together.
+  // It's OK to play some guessing game here: if we locate `package.json`
+  // with a package name and version, we'll use it; otherwise, we'll fallback
+  // to 'default'.
+  for (const directory of directories) {
+    const packageJson = path.resolve(directory, '..', 'package.json');
+    if (fs.existsSync(packageJson)) {
+      const json = JSON.parse((await readFile(packageJson)).toString()) as {
+        name: string;
+        version: string;
+      };
+      const name = json.name.replace(/[^\w\d]/g, '_');
+      const version = json.version.replace(/[^\w\d]/g, '_');
+      const hopefullyUniqueName = `${name}_${version}_protos`;
+      return hopefullyUniqueName;
+    }
+  }
+  return 'default';
+}
+
 /**
  * Main function. Takes an array of directories to process.
  * Looks for JSON files matching `PROTO_LIST_REGEX`, parses them to get a list of all
@@ -267,8 +301,9 @@ export async function main(directories: string[]): Promise<void> {
   for (const directory of directories) {
     protoJsonFiles.push(...(await findProtoJsonFiles(directory)));
   }
+  const rootName = await generateRootName(directories);
   const protos = await buildListOfProtos(protoJsonFiles);
-  await compileProtos(protos);
+  await compileProtos(rootName, protos);
 }
 
 /**
@@ -280,12 +315,12 @@ function usage() {
     `Finds all files matching ${PROTO_LIST_REGEX} in the given directories.`
   );
   console.log(
-    `Each of those files should contain a JSON array of proto files used by the`
+    'Each of those files should contain a JSON array of proto files used by the'
   );
   console.log(
-    `client library. Those proto files will be compiled to JSON using pbjs tool`
+    'client library. Those proto files will be compiled to JSON using pbjs tool'
   );
-  console.log(`from protobufjs.`);
+  console.log('from protobufjs.');
 }
 
 if (require.main === module) {