feat: Add typescript declaration file gen support via pbts (#5)

* Add typescript declaration file gen support via pbts

* Fix specs in CI, add test case

* Add nodejs github workflow

* Remove unnecessary github workflow

* Add package-lock.json

* Add webpack's memfs to compile return value

* Some style and typing fixes in main file

* Tests for basic pbts functionality

* Fix linting issues

* Disallow pbts + json at the schema level, fixes in main file

* Spec fixes and schema improvements

* Fix lint issues

* No need to export filesystem, actually

* Process additional pbts arguments

* Add specs for additional pbts use cases

* Style fix

* README tweak

Co-authored-by: Joshua Cole <[email protected]>
Co-authored-by: Kevin Montag <[email protected]>

README.md

@@ -31,23 +31,40 @@ module.exports = {
             use: {
               loader: 'protobufjs-loader',
               options: {
-                /* controls the "target" flag to pbjs - true for
+                /* Controls the "target" flag to pbjs - true for
                  * json-module, false for static-module.
+                 *
                  * default: false
                  */
-                json: true,
+                json: false,
 
-                /* import paths provided to pbjs.
+                /* Import paths provided to pbjs.
+                 *
                  * default: webpack import paths (i.e. config.resolve.modules)
                  */
                 paths: ['/path/to/definitions'],
 
-                /* additional command line arguments passed to
-                 * pbjs, see https://github.com/dcodeIO/ProtoBuf.js/#pbjs-for-javascript
-                 * for a list of what's available.
+                /* Additional command line arguments passed to pbjs.
+                 *
                  * default: []
                  */
-                pbjsArgs: ['--no-encode']
+                pbjsArgs: ['--no-encode'],
+
+                /* Enable Typescript declaration file generation via pbts.
+                 *
+                 * Declaration files will be written every time the loader runs.
+                 * They'll be saved in the same directory as the protobuf file
+                 * being processed, with a `.d.ts` extension.
+                 *
+                 * This can be a config object or a boolean.
+                 *
+                 * default: false
+                 */
+                pbts: {
+                  /* Additional command line arguments passed to pbts.
+                   */
+                  args: ['--no-comments'],
+                }
               }
             }
         }]

index.js

@@ -1,5 +1,5 @@
 const fs = require('fs');
-const { pbjs } = require('protobufjs/cli');
+const { pbjs, pbts } = require('protobufjs/cli');
 const protobuf = require('protobufjs');
 const tmp = require('tmp-promise');
 const validateOptions = require('schema-utils').validate;
@@ -12,17 +12,67 @@ const schema = {
   properties: {
     json: {
       type: 'boolean',
+      default: false,
     },
     paths: {
       type: 'array',
     },
     pbjsArgs: {
       type: 'array',
+      default: [],
+    },
+    pbts: {
+      oneOf: [
+        {
+          type: 'boolean',
+        },
+        {
+          type: 'object',
+          properties: {
+            args: {
+              type: 'array',
+              default: [],
+            },
+          },
+          additionalProperties: false,
+        },
+      ],
+      default: false,
     },
   },
+
+  // pbts config is only applicable if the pbjs target is
+  // `static-module`, i.e. if the `json` flag is false. We enforce
+  // this at the schema level; see
+  // https://json-schema.org/understanding-json-schema/reference/conditionals.html#implication.
+  anyOf: [
+    {
+      properties: {
+        json: { const: true },
+        pbts: { const: false },
+      },
+    },
+    {
+      not: {
+        properties: { json: { const: true } },
+      },
+    },
+  ],
   additionalProperties: false,
 };
 
+/**
+ * Shared type for the validated options object, with no missing
+ * properties (i.e. the user-provided object merged with default
+ * values).
+ *
+ * @typedef {{ args: string[] }} PbtsOptions
+ * @typedef {{
+ *   json: boolean, paths: string[], pbjsArgs: string[],
+ *   pbts: boolean | PbtsOptions
+ * }} LoaderOptions
+ */
+
 /**
  * We're supporting multiple webpack versions, so there are several
  * different possible structures for the `this` context in our loader
@@ -31,12 +81,47 @@ const schema = {
  * The `never` generic in the v5 context sets the return type of
  * `getOptions`. Since we're using the deprecated `loader-utils`
  * method of fetching options, this should be fine; however, if we
- * drop support for older webpack versions, we'll want to define a
- * stricter type for the options object.
+ * drop support for older webpack versions, we'll want to switch to
+ * using `getOptions`.
  *
  * @typedef { import('webpack').LoaderContext<never> | import('webpack4').loader.LoaderContext | import('webpack3').loader.LoaderContext | import('webpack2').loader.LoaderContext } LoaderContext
  */
 
+/** @type { (resourcePath: string, pbtsOptions: true | PbtsOptions, compiledContent: string, callback: NonNullable<ReturnType<LoaderContext['async']>>) => any } */
+const execPbts = (resourcePath, pbtsOptions, compiledContent, callback) => {
+  /** @type PbtsOptions */
+  const normalizedOptions = {
+    args: [],
+    ...(pbtsOptions === true ? {} : pbtsOptions),
+  };
+
+  // pbts CLI only supports streaming from stdin without a lot of
+  // duplicated logic, so we need to use a tmp file. :(
+  tmp
+    .file({ postfix: '.js' })
+    .then(
+      (o) =>
+        new Promise((resolve, reject) => {
+          fs.write(o.fd, compiledContent, (err) => {
+            if (err) {
+              reject(err);
+            } else {
+              resolve(o.path);
+            }
+          });
+        })
+    )
+    .then((compiledFilename) => {
+      const declarationFilename = `${resourcePath}.d.ts`;
+      const pbtsArgs = ['-o', declarationFilename]
+        .concat(normalizedOptions.args)
+        .concat([compiledFilename]);
+      pbts.main(pbtsArgs, (err) => {
+        callback(err, compiledContent);
+      });
+    });
+};
+
 /** @type { (this: LoaderContext, source: string) => any } */
 module.exports = function protobufJsLoader(source) {
   const callback = this.async();
@@ -64,17 +149,25 @@ module.exports = function protobufJsLoader(source) {
     return undefined;
   })();
 
-  /** @type {{ json: boolean, paths: string[], pbjsArgs: string[] }} */
+  /** @type LoaderOptions */
   const options = {
     json: false,
 
     // Default to the paths given to the compiler.
     paths: defaultPaths || [],
 
     pbjsArgs: [],
+
+    pbts: false,
+
     ...getOptions(this),
   };
-  validateOptions(schema, options, { name: 'protobufjs-loader' });
+  try {
+    validateOptions(schema, options, { name: 'protobufjs-loader' });
+  } catch (err) {
+    callback(err instanceof Error ? err : new Error(`${err}`), undefined);
+    return;
+  }
 
   /** @type { string } */
   let filename;
@@ -161,7 +254,11 @@ module.exports = function protobufJsLoader(source) {
             callback(depErr);
           })
           .then(() => {
-            callback(err, result);
+            if (!options.pbts || err) {
+              callback(err, result);
+            } else {
+              execPbts(self.resourcePath, options.pbts, result || '', callback);
+            }
           });
       });
     });

package.json

@@ -29,9 +29,11 @@
   },
   "devDependencies": {
     "@types/chai": "^4.3.1",
+    "@types/glob": "^7.2.0",
     "@types/loader-utils": "^2.0.3",
     "@types/memory-fs": "^0.3.3",
     "@types/mocha": "^8.2.3",
+    "@types/tmp": "^0.2.3",
     "@types/webpack2": "npm:@types/webpack@^2.0.0",
     "@types/webpack3": "npm:@types/webpack@^3.0.0",
     "@types/webpack4": "npm:@types/webpack@^4.0.0",

test/helpers/compile.js

@@ -34,7 +34,7 @@ const isWebpack5 =
  * @typedef {{ arguments: [string], context: import('../../index').LoaderContext, options: never }} InspectLoaderResult
  */
 
-/** @type { (fixture: string, loaderOpts?: object, webpackOpts?: object) => Promise<InspectLoaderResult> } */
+/** @type { (fixture: string, loaderOpts?: object, webpackOpts?: object) => Promise<{ inspect: InspectLoaderResult }> } */
 module.exports = function compile(fixture, loaderOpts, webpackOpts) {
   return new Promise((resolve, reject) => {
     /** @type { InspectLoaderResult } */
@@ -108,6 +108,20 @@ module.exports = function compile(fixture, loaderOpts, webpackOpts) {
         }
         if (stats) {
           if (stats.hasErrors()) {
+            if ('compilation' in stats) {
+              /** @type Error */
+              // The `stats` object appears to be incorrectly typed;
+              // this compilation field exists in practice.
+              //
+              // @ts-ignore
+              const compilationErr = stats.compilation.errors[0];
+              if (compilationErr) {
+                return compilationErr;
+              }
+            }
+
+            // fallback in case no specific error was found above for
+            // some reason.
             return 'compilation error';
           }
           if (stats.hasWarnings()) {
@@ -120,7 +134,9 @@ module.exports = function compile(fixture, loaderOpts, webpackOpts) {
       if (problem) {
         reject(problem);
       } else {
-        resolve(inspect);
+        resolve({
+          inspect,
+        });
       }
     });
   });