feat: remove json config key in favor of generic target string

BREAKING CHANGE: The `json` config option is no longer recognized. The pbjs `--target` value can now be specified explicitly using the `target` config option. If you were using the `json` option, you can migrate your config by replacing `json: true` with `target: 'static-module'`, and `json: false` with `target: 'json-module'`. The default behavior has also been preserved, so if you weren't using the `json` option, no changes are necessary.
kmontag · Jul 18, 2022 · 8344a47 · 8344a47
1 parent fa84b9c
commit 8344a47
Show file tree

Hide file tree

Showing 2 changed files with 49 additions and 52 deletions.
diff --git a/index.js b/index.js
@@ -6,13 +6,15 @@ const validateOptions = require('schema-utils').validate;
 
 const { getOptions } = require('loader-utils');
 
+const TARGET_STATIC_MODULE = 'static-module';
+
 /** @type { Parameters<typeof validateOptions>[0] } */
 const schema = {
   type: 'object',
   properties: {
-    json: {
-      type: 'boolean',
-      default: false,
+    target: {
+      type: 'string',
+      default: TARGET_STATIC_MODULE,
     },
     paths: {
       type: 'array',
@@ -41,23 +43,6 @@ const schema = {
     },
   },
 
-  // 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,
 };
 
@@ -68,8 +53,9 @@ const schema = {
  *
  * @typedef {{ args: string[] }} PbtsOptions
  * @typedef {{
- *   json: boolean, paths: string[], pbjsArgs: string[],
- *   pbts: boolean | PbtsOptions
+ *   paths: string[], pbjsArgs: string[],
+ *   pbts: boolean | PbtsOptions,
+ *   target: string,
  * }} LoaderOptions
  */
 
@@ -152,7 +138,7 @@ module.exports = function protobufJsLoader(source) {
 
     /** @type LoaderOptions */
     const options = {
-      json: false,
+      target: TARGET_STATIC_MODULE,
 
       // Default to the paths given to the compiler.
       paths: defaultPaths || [],
@@ -235,13 +221,12 @@ module.exports = function protobufJsLoader(source) {
           });
         });
 
-        let args = options.pbjsArgs;
+        /** @type { string[] } */
+        let args = ['-t', options.target];
         paths.forEach((path) => {
           args = args.concat(['-p', path]);
         });
-        args = args
-          .concat(['-t', options.json ? 'json-module' : 'static-module'])
-          .concat([filename]);
+        args = args.concat(options.pbjsArgs).concat([filename]);
 
         pbjs.main(args, (err, result) => {
           // Make sure we've added all dependencies before completing.

diff --git a/test/index.test.js b/test/index.test.js
@@ -50,7 +50,7 @@ describe('protobufjs-loader', function () {
   describe('with JSON / reflection', function () {
     beforeEach(function () {
       this.opts = {
-        json: true,
+        target: 'json-module',
       };
     });
     it('should compile to a JSON representation', function (done) {
@@ -74,7 +74,7 @@ describe('protobufjs-loader', function () {
     });
 
     it('should compile static code when the option is set explicitly', function (done) {
-      compile('basic', { json: false }).then(({ inspect }) => {
+      compile('basic', { target: 'static-module' }).then(({ inspect }) => {
         const contents = minify(inspect.arguments[0]);
         assert.include(contents, 'foo.Bar=function(){');
         done();
@@ -152,25 +152,6 @@ describe('protobufjs-loader', function () {
         });
       });
 
-      it('should fail if typescript compilation is enabled for JSON output', function (done) {
-        compile('basic', {
-          json: true,
-          pbts: true,
-        }).catch((compilationErr) => {
-          assert.include(
-            `${compilationErr}`,
-            'configuration.pbts should be equal to constant false'
-          );
-          glob(path.join(this.tmpDir, '*.d.ts'), (err, files) => {
-            if (err) {
-              throw err;
-            }
-            assert.equal(0, files.length);
-            done();
-          });
-        });
-      });
-
       it('should compile typescript when enabled', function (done) {
         compile(path.join(this.tmpDir, 'basic'), { pbts: true }).then(() => {
           glob(path.join(this.tmpDir, '*.d.ts'), (globErr, files) => {
@@ -196,6 +177,37 @@ describe('protobufjs-loader', function () {
         });
       });
 
+      it('should compile nearly-empty declarations if typescript compilation is enabled for JSON output', function (done) {
+        compile(path.join(this.tmpDir, 'basic'), {
+          target: 'json-module',
+          pbts: true,
+        }).then(() => {
+          glob(path.join(this.tmpDir, '*.d.ts'), (err, files) => {
+            if (err) {
+              throw err;
+            }
+
+            const expectedDefinitionsFile = path.join(
+              this.tmpDir,
+              'basic.proto.d.ts'
+            );
+            assert.sameMembers([expectedDefinitionsFile], files);
+
+            fs.readFile(expectedDefinitionsFile, (readErr, content) => {
+              if (readErr) {
+                throw readErr;
+              }
+              const declarations = content.toString();
+              assert.equal(
+                declarations.trim(),
+                'import * as $protobuf from "protobufjs";'
+              );
+              done();
+            });
+          });
+        });
+      });
+
       it('should pass arguments to pbts', function (done) {
         compile(path.join(this.tmpDir, 'basic'), {
           pbts: {
@@ -303,7 +315,7 @@ describe('protobufjs-loader', function () {
       compile(
         'import',
         {
-          json: true,
+          target: 'json-module',
         },
         {
           resolve: {
@@ -320,7 +332,7 @@ describe('protobufjs-loader', function () {
     it('should respect an explicit paths configuration', function (done) {
       const { innerString } = this;
       compile('import', {
-        json: true,
+        target: 'json-module',
         paths: [path.resolve(__dirname, 'fixtures')],
       }).then(({ inspect }) => {
         const contents = minify(inspect.arguments[0]);
@@ -348,7 +360,7 @@ describe('protobufjs-loader', function () {
 
     it('should fail when the import is not found', function (done) {
       compile('import', {
-        json: true,
+        target: 'json-module',
         // No include paths provided, so the 'import' fixture should
         // fail to compile.
       }).catch((err) => {
@@ -364,7 +376,7 @@ describe('protobufjs-loader', function () {
   describe('with invalid options', function () {
     it('should fail if unreconized properties are added', function (done) {
       compile('basic', {
-        json: true,
+        target: 'json-module',
         foo: true,
       }).catch((err) => {
         assert.include(`${err}`, "configuration has an unknown property 'foo'");