Update NodeJS packaging patterns.

- retire use_pbjs flag -- it's not used anymore, and it does not
  fit with the new pattern of Gapic code.
- update the grpc package template: this fits with the proto packages
  used by gcloud-node.
- update gax package template: this will work well with the new
  pattern of Gapic code, as you can see in googleapis/gapic-generator#392
  or googleapis/google-cloud-node#1476

Author: jmuk

bin/gen-api-package

@@ -197,18 +197,6 @@ var parseArgs = function parseArgs() {
       dest: 'altJava'
     }
   );
-  cli.addArgument(
-    [ '--nodejs_use_pbjs' ],
-    {
-      defaultValue: false,
-      action: 'storeTrue',
-      help: 'When set, the nodejs grpc client is generated with the json data\n'
-            + ' generated from pbjs. Otherwise the client bundles .proto\n'
-            + ' files. Note that pbjs-based client does not work currently\n'
-            + ' (see https://github.com/googleapis/packman/issues/35).',
-      dest: 'nodejsUsePbjs'
-    }
-  );
   cli.addArgument(
     [ '--override_plugins' ],
     {

lib/api_repo.js

@@ -24,14 +24,11 @@ var fs = require('fs-extra');
 var glob = require('glob');
 var packager = require('./packager');
 var path = require('path');
-var pbjs = require('protobufjs/cli/pbjs');
-var pbjsUtil = require('protobufjs/cli/pbjs/util');
 var readline = require('readline');
 var request = require('request');
 var tmp = require('tmp');
 
 var EventEmitter = require('events').EventEmitter;
-var ProtoBuf = require('protobufjs');
 var StreamZip = require('node-stream-zip');
 
 exports.ApiRepo = ApiRepo;
@@ -214,7 +211,7 @@ ApiRepo.prototype.buildPackages =
               packageInfo: that.packageInfo,
               generated: generated
             }, templateInfo[l]);
-            if (l === 'nodejs' && !that.opts.nodejsUsePbjs) {
+            if (l === 'nodejs') {
               opts.packageInfo.protoFiles = _.filter(generated, function(file) {
                 return file.match(/\.proto$/);
               });
@@ -594,22 +591,7 @@ ApiRepo.prototype._buildProtos =
           var fullPathProtos = [];
           function makeModule() {
             var includePath = _.union(that.includePath, that.repoDirs);
-            if (!that.opts.nodejsUsePbjs) {
-              makeProtoBasedNodeModule(fullPathProtos, includePath);
-              return;
-            }
-            var opts = {
-              root: that.repoDirs,
-              source: 'proto',
-              path: includePath
-            };
-
-            var builder = loadProtos(fullPathProtos, opts);
-            var outDir = path.join(that.outDir, language);
-            fs.mkdirsSync(outDir);
-            var servicePath = path.join(outDir, 'service.js');
-            var commonJS = pbjs.targets.commonjs(builder, opts);
-            fs.writeFile(servicePath, commonJS, findOutputs);
+            makeProtoBasedNodeModule(fullPathProtos, includePath);
           }
 
           this._findProtos(
@@ -619,7 +601,6 @@ ApiRepo.prototype._buildProtos =
                 next();
               }
           );
-          makeModule();
         }.bind(this);
         this._findProtos(name, version, makeNodeModule);
       } else {
@@ -1033,41 +1014,3 @@ ApiRepo.prototype._makeProtocFunc = function _makeProtocFunc(opts, language) {
 
   return callProtoc;
 };
-
-/**
- * Helps construct a JSON representation each proto file for the nodejs build.
- *
- * @param {string[]} filenames - The list of files.
- * @param {object} opts - provides configuration info
- * @param {object} opts.root - a virtual root folder that contains all the protos
- * @param {object} opts.path - an array of folders where other protos reside
- *
- * @return {object} a ProtoBuf.Builder containing loaded representations of the
- * protos
- */
-function loadProtos(filenames, opts) {
-  opts = opts || [];
-  var builder = ProtoBuf.newBuilder();
-  var loaded = [];
-  builder.importRoot = opts.root;
-  filenames.forEach(function(filename) {
-    var data = pbjs.sources.proto.load(filename, opts, loaded);
-    builder.import(data, filename);
-  });
-  builder.resolveAll();
-  return builder;
-}
-
-/**
- * Replace isDescriptor with a version that always returns false.
- *
- * pbjs/util.isDescriptor excludes imports that in google/protobuf.
- *
- * However, the nodejs packages need to be self-contained, so we actually want
- * to include these.
- * @param {string} name - The name of the message.
- * @return {Boolean} Whether it is a descriptor or not.
- */
-pbjsUtil.isDescriptor = function(name) {
-  return false;
-};

lib/packager.js

@@ -366,9 +366,6 @@ function makeNodejsPackage(opts, done) {
   opts = _.merge({}, settings.nodejs, opts);
   var tasks = [];
 
-  if (!opts.packageInfo.nodejsUsePbjs) {
-    opts.templates.push('service.js.mustache');
-  }
   // Move copyable files to the top-level dir.
   opts.copyables.forEach(function(f) {
     var src = path.join(opts.templateDir, f);
@@ -384,7 +381,10 @@ function makeNodejsPackage(opts, done) {
     var dstBase = removeMustacheExt(f);
     var tmpl = path.join(opts.templateDir, f);
     var dst = path.join(opts.top, dstBase);
-    tasks.push(expand(tmpl, dst, opts.packageInfo));
+    // index.js should reside among other .js files for GAX packages.
+    if (dstBase !== 'index.js') {
+      tasks.push(expand(tmpl, dst, opts.packageInfo));
+    }
   });
 
   function runTask(err, jsFiles) {
@@ -397,13 +397,22 @@ function makeNodejsPackage(opts, done) {
       return;
     }
     opts.packageInfo.apiFiles = _.map(jsFiles, function(jsFile) {
-      var filePath = jsFile.replace(/\.js$/, '');
+      var filePath = path.basename(jsFile, '.js');
       return {
         name: underscoreToLowerCamelCase(filePath),
         filePath: filePath
       };
     });
-    opts.packageInfo.serviceAddressName = opts.packageInfo.apiFiles[0].name;
+    if (opts.packageInfo.apiFiles.length > 0) {
+      _.last(opts.packageInfo.apiFiles).last = true;
+      opts.packageInfo.serviceAddressName = opts.packageInfo.apiFiles[0].name;
+      // Creating a task to bring index.js into the same directory where
+      // other generated .js files exist.
+      tasks.push(expand(path.join(opts.templateDir, 'index.js.mustache'),
+                        path.join(path.dirname(jsFiles[0]), 'index.js'),
+                        opts.packageInfo));
+    }
+    opts.packageInfo.singleFile = (opts.packageInfo.apiFiles.length === 1);
     async.series(tasks, function(err) {
       if (!err && opts.copyables.indexOf('PUBLISHING.md') !== -1) {
         console.log('The nodejs package', pkgName(opts.packageInfo),
@@ -418,7 +427,9 @@ function makeNodejsPackage(opts, done) {
   // find lib/*.js files for gax API packages to generate index.js properly.
   if (opts.mockApiFilesForTest) {
     // For tests, mock data is passed instead of scanning the filesystem.
-    runTask(null, opts.mockApiFilesForTest);
+    runTask(null, _.map(opts.mockApiFilesForTest, function(file) {
+      return path.join(opts.top, 'src', file);
+    }));
   } else {
     glob.glob('*.js', {cwd: path.join(opts.top, 'src')}, runTask);
   }

templates/gax/nodejs/index.js.mustache

@@ -16,24 +16,39 @@
 'use strict';
 
 {{#apiFiles}}
-var {{{name}}} = require('./src/{{{filePath}}}');
+var {{{name}}} = require('./{{{filePath}}}');
 {{/apiFiles}}
 var gax = require('google-gax');
+var extend = require('extend');
+{{^singleFile}}
+var lodash = require('lodash');
+{{/singleFile}}
 
-module.exports = function(options) {
-  options = options || {};
-  if (!('scopes' in options)) {
-    options.scopes = [];
-    {{#apiFiles}}
-    options.scopes.push({{{name}}}.ALL_SCOPES);
-    {{/apiFiles}}
-  }
+function {{{api.version}}}(options) {
+  options = extend({
+    scopes: {{{api.version}}}.ALL_SCOPES
+  }, options);
   var gaxGrpc = gax.grpc(options);
+  {{#singleFile}}
+  return {{{apiFiles[0].name}}}(gaxGrpc);
+  {{/singleFile}}
+  {{^singleFile}}
   var result = {};
   {{#apiFiles}}
   extend(result, {{{name}}}(gaxGrpc));
   {{/apiFiles}}
   return result;
+  {{/singleFile}}
 }
-
-module.exports.SERVICE_ADDRESS = {{{serviceAddressName}}}.SERVICE_ADDRESS;
+{{{api.version}}}.SERVICE_ADDRESS = {{{serviceAddressName}}}.SERVICE_ADDRESS;
+{{#singleFile}}
+{{{api.version}}}.ALL_SCOPES = {{{serviceAddressName}}}.ALL_SCOPES;
+{{/singleFile}}
+{{^singleFile}}
+{{{api.version}}}.ALL_SCOPES = lodash.union(
+{{#apiFiles}}
+  {{{name}}}.ALL_SCOPES{{^last}},{{/last}}
+{{/apiFiles}}
+);
+{{/singleFile}}
+module.exports = {{{api.version}}};

templates/gax/nodejs/package.json.mustache

@@ -11,5 +11,5 @@
     "grpc": "^{{{dependencies.grpc.nodejs.version}}}",
     "{{{api.dependsOn}}}-{{{api.version}}}": "^{{{api.semantic_version}}}"
   },
-  "main": "index.js"
+  "main": "src/index.js"
 }

templates/nodejs/package.json.mustache

@@ -8,9 +8,7 @@
   "main": "index.js",
   "files": [
     "LICENSE",
-{{#nodejsUseProtos}}
     "proto/**/*",
-{{/nodejsUseProtos}}
     "index.js"
   ]
 }

templates/nodejs/service.js.mustache

@@ -100,10 +100,8 @@ describe('ApiRepo', function() {
   describe('on the test fixture repo with no plugins', function() {
     var fakes, repo; // eslint-disable-line
     describe('configured for nodejs', function() {
-      // TODO: add a test case for nodejsUsePbjs: false.
       beforeEach(function() {
         repo = new ApiRepo({
-          nodejsUsePbjs: true,
           includePath: [path.join(__dirname, 'fixtures', 'include')],
           languages: ['nodejs'],
           templateRoot: path.join(__dirname, '..', 'templates')

test/api_repo.js

@@ -13,12 +13,27 @@
  * See the License for the specific language governing permissions and
  * limitations under the License.
  */
+'use strict';
 
-var apiClient = require('./src/foo_api');
-for (var key in apiClient) {
-  exports[key] = apiClient[key];
-}
-var apiClient = require('./src/bar_api');
-for (var key in apiClient) {
-  exports[key] = apiClient[key];
+var fooApi = require('./foo_api');
+var barApi = require('./bar_api');
+var gax = require('google-gax');
+var extend = require('extend');
+var lodash = require('lodash');
+
+function v2(options) {
+  options = extend({
+    scopes: v2.ALL_SCOPES
+  }, options);
+  var gaxGrpc = gax.grpc(options);
+  var result = {};
+  extend(result, fooApi(gaxGrpc));
+  extend(result, barApi(gaxGrpc));
+  return result;
 }
+v2.SERVICE_ADDRESS = fooApi.SERVICE_ADDRESS;
+v2.ALL_SCOPES = lodash.union(
+  fooApi.ALL_SCOPES,
+  barApi.ALL_SCOPES
+);
+module.exports = v2;

test/fixtures/gax/nodejs/index.js test/fixtures/gax/nodejs/src/v2/index.jstest/fixtures/gax/nodejs/index.js renamed to test/fixtures/gax/nodejs/src/v2/index.js

@@ -5,15 +5,10 @@
   "author": "Google Inc.",
   "description": "GRPC library for service packager-unittest-v2",
   "license": "BSD-3-Clause",
-  "dependencies": {
-    "grpc": "^0.10.0",
-    "protobufjs": "^5.0.1",
-    "google-auth-library": "^0.9.2"
-  },
   "main": "index.js",
   "files": [
     "LICENSE",
-    "index.js",
-    "service.js"
+    "proto/**/*",
+    "index.js"
   ]
 }