Repository: stoplightio/spectral
Pull request #2174 (2022-06-03 19:55:58)

fix(ruleset-bundler): never externalize builtins
Fixes the 2nd case in #2107 

**Checklist**

- [x] Tests added / updated
- [x] Docs added / updated

**Does this PR introduce a breaking change?**

- [ ] Yes
- [x] No


**Additional context**

This is somewhat similar to https://github.com/stoplightio/spectral/pull/2109 with that difference we alter the default resolution only if the builtins plugin is registered. Otherwise, externalization is not modified.


Patch:
diff --git a/packages/ruleset-bundler/package.json b/packages/ruleset-bundler/package.json
index 4f741f19..dc7ed967 100644
--- a/packages/ruleset-bundler/package.json
+++ b/packages/ruleset-bundler/package.json
@@ -38,7 +38,7 @@
     "release": "semantic-release -e semantic-release-monorepo"
   },
   "dependencies": {
-    "@rollup/plugin-commonjs": "^21.0.1",
+    "@rollup/plugin-commonjs": "~22.0.0",
     "@stoplight/path": "1.3.2",
     "@stoplight/spectral-core": ">=1",
     "@stoplight/spectral-formats": ">=1",
@@ -51,7 +51,7 @@
     "@stoplight/types": "^12.3.0",
     "@types/node": "*",
     "pony-cause": "1.1.1",
-    "rollup": "~2.67.0",
+    "rollup": "~2.75.5",
     "tslib": "^2.3.1",
     "validate-npm-package-name": "3.0.0"
   },
diff --git a/packages/ruleset-bundler/src/plugins/builtins.ts b/packages/ruleset-bundler/src/plugins/builtins.ts
index 937a831c..32d3d8fa 100644
--- a/packages/ruleset-bundler/src/plugins/builtins.ts
+++ b/packages/ruleset-bundler/src/plugins/builtins.ts
@@ -5,7 +5,7 @@ import * as parsers from '@stoplight/spectral-parsers';
 import * as refResolver from '@stoplight/spectral-ref-resolver';
 import * as rulesets from '@stoplight/spectral-rulesets';
 import * as runtime from '@stoplight/spectral-runtime';
-import type { Plugin } from 'rollup';
+import type { Plugin, InputOptions } from 'rollup';
 
 type Module = 'core' | 'formats' | 'functions' | 'parsers' | 'ref-resolver' | 'rulesets' | 'runtime';
 type GlobalModules = Record<`@stoplight/spectral-${Module}`, string>;
@@ -49,6 +49,20 @@ export const builtins = (overrides: Partial<Overrides> = {}): Plugin => {
 
   return {
     name: NAME,
+    options(rawOptions): InputOptions {
+      const external = rawOptions.external;
+
+      if (typeof external === 'function') {
+        return {
+          ...rawOptions,
+          external: <typeof external>(
+            ((id, importer, isResolved) => !(id in modules) && external(id, importer, isResolved))
+          ),
+        };
+      }
+
+      return rawOptions;
+    },
     resolveId(id): string | null {
       if (id in modules) {
         return id;
diff --git a/yarn.lock b/yarn.lock
index b5cf069a..7197c8a7 100644
--- a/yarn.lock
+++ b/yarn.lock
@@ -2077,9 +2077,9 @@ __metadata:
   languageName: node
   linkType: hard
 
-"@rollup/plugin-commonjs@npm:^21.0.1":
-  version: 21.0.1
-  resolution: "@rollup/plugin-commonjs@npm:21.0.1"
+"@rollup/plugin-commonjs@npm:~22.0.0":
+  version: 22.0.0
+  resolution: "@rollup/plugin-commonjs@npm:22.0.0"
   dependencies:
     "@rollup/pluginutils": ^3.1.0
     commondir: ^1.0.1
@@ -2089,8 +2089,8 @@ __metadata:
     magic-string: ^0.25.7
     resolve: ^1.17.0
   peerDependencies:
-    rollup: ^2.38.3
-  checksum: 3e56be58c72d655face6f361f85923ddcc3cc07760b5a3a91cfc728115dfed358fc595781148c512d53a03be8c703133379f228e78fd2aed8655fae9d83800b6
+    rollup: ^2.68.0
+  checksum: fdcce2bf58875fde0e06f001544c0d9a0509a12929393862f72dcef8fcbf4d5d0ba0d5db6cf10ba4351335caf67a3dbdb95000678c468585e3972994f92e2ce9
   languageName: node
   linkType: hard
 
@@ -2484,7 +2484,7 @@ __metadata:
   version: 0.0.0-use.local
   resolution: "@stoplight/spectral-ruleset-bundler@workspace:packages/ruleset-bundler"
   dependencies:
-    "@rollup/plugin-commonjs": ^21.0.1
+    "@rollup/plugin-commonjs": ~22.0.0
     "@stoplight/path": 1.3.2
     "@stoplight/spectral-core": ">=1"
     "@stoplight/spectral-formats": ">=1"
@@ -2501,7 +2501,7 @@ __metadata:
     memfs: ^3.3.0
     pony-cause: 1.1.1
     prettier: ^2.4.1
-    rollup: ~2.67.0
+    rollup: ~2.75.5
     tslib: ^2.3.1
     validate-npm-package-name: 3.0.0
   languageName: unknown
@@ -11202,9 +11202,9 @@ __metadata:
   languageName: node
   linkType: hard
 
-"rollup@npm:~2.67.0":
-  version: 2.67.2
-  resolution: "rollup@npm:2.67.2"
+"rollup@npm:~2.75.5":
+  version: 2.75.5
+  resolution: "rollup@npm:2.75.5"
   dependencies:
     fsevents: ~2.3.2
   dependenciesMeta:
@@ -11212,7 +11212,7 @@ __metadata:
       optional: true
   bin:
     rollup: dist/bin/rollup
-  checksum: 9aca5251ba4b441064183cde2394b91567259002d68086bdd3906db66d55dd148ab27e57c51eb53830d7b9b813c2d4e834b7735d65e2a869780bc639d4a20c38
+  checksum: fa3e61959efbc88cda75315dc22f035489be9dda3ae7d45a3a474f19efc3661ef6f9849486c7ca51e1a3e2aedef91f4e3223e8123bbeaf155913533d071994d1
   languageName: node
   linkType: hard
 


Test patch:
diff --git a/packages/ruleset-bundler/src/__tests__/index.jest.test.ts b/packages/ruleset-bundler/src/__tests__/index.jest.test.ts
index 9afc25fe..879f269e 100644
--- a/packages/ruleset-bundler/src/__tests__/index.jest.test.ts
+++ b/packages/ruleset-bundler/src/__tests__/index.jest.test.ts
@@ -8,6 +8,7 @@ import { browser } from '../presets/browser';
 import { commonjs } from '../plugins/commonjs';
 import { virtualFs } from '../plugins/virtualFs';
 import { runtime } from '../presets/runtime';
+import { builtins } from '../plugins/builtins';
 
 jest.mock('fs');
 
@@ -97,4 +98,50 @@ var spectral = {
 
 export { spectral as default };`);
   });
+
+  it('given node target, should support commonjs for remote ruleset with builtin modules', async () => {
+    serveAssets({
+      'https://tmp/input.js': `var spectralFormats = require('@stoplight/spectral-formats');
+var spectralFunctions = require('@stoplight/spectral-functions');
+const ruleset = {
+  rules: {
+    'my-rule': {
+      given: '$',
+      then: {
+        function: spectralFunctions.schema,
+        functionOptions: {
+          schema: {
+            type: 'object',
+          },
+        },
+      },
+    },
+  },
+};
+module.exports = ruleset;
+`,
+    });
+
+    const code = await bundleRuleset('https://tmp/input.js', {
+      target: 'node',
+      format: 'commonjs',
+      plugins: [builtins(), commonjs(), ...node({ fs, fetch }), virtualFs(io)],
+    });
+
+    expect(code).toContain(`const ruleset = {
+  rules: {
+    'my-rule': {
+      given: '$',
+      then: {
+        function: spectralFunctions.schema,
+        functionOptions: {
+          schema: {
+            type: 'object',
+          },
+        },
+      },
+    },
+  },
+};`);
+  });
 });
diff --git a/packages/ruleset-bundler/src/plugins/__tests__/builtins.spec.ts b/packages/ruleset-bundler/src/plugins/__tests__/builtins.spec.ts
index 44f3fc9b..e95ac142 100644
--- a/packages/ruleset-bundler/src/plugins/__tests__/builtins.spec.ts
+++ b/packages/ruleset-bundler/src/plugins/__tests__/builtins.spec.ts
@@ -31,7 +31,7 @@ describe('Builtins Plugin', () => {
     randomSpy.mockRestore();
   });
 
-  describe.each<BundleOptions['target']>(['browser', 'runtime'])('given %s target', target => {
+  describe.each<BundleOptions['target']>(['browser', 'node', 'runtime'])('given %s target', target => {
     it('should inline Spectral packages & expose it to the runtime', async () => {
       serveAssets({
         '/tmp/input.js': `import { schema } from '@stoplight/spectral-functions';
@@ -199,62 +199,4 @@ readFile();`,
       });
     });
   });
-
-  describe('given node target', () => {
-    it('should be a no-op', async () => {
-      serveAssets({
-        '/tmp/input.js': `import { schema } from '@stoplight/spectral-functions';
-import { oas } from '@stoplight/spectral-rulesets';
-
-export default {
-  extends: [oas],
-  rules: {
-    'my-rule': {
-      given: '$',
-      then: {
-        function: schema,
-        functionOptions: {
-          schema: {
-            type: 'object',
-          },
-        },
-      },
-    },
-  },
-};`,
-      });
-
-      const code = await bundleRuleset('/tmp/input.js', {
-        target: 'node',
-        plugins: [builtins(), virtualFs(io)],
-      });
-
-      expect(code).toEqual(`import { schema } from '@stoplight/spectral-functions';
-import { oas } from '@stoplight/spectral-rulesets';
-
-var input = {
-  extends: [oas],
-  rules: {
-    'my-rule': {
-      given: '$',
-      then: {
-        function: schema,
-        functionOptions: {
-          schema: {
-            type: 'object',
-          },
-        },
-      },
-    },
-  },
-};
-
-export { input as default };
-`);
-
-      expect(
-        globalThis[Symbol.for('@stoplight-spectral/builtins')]['822928']['@stoplight/spectral-functions'],
-      ).toStrictEqual(functions);
-    });
-  });
 });
diff --git a/packages/ruleset-bundler/src/plugins/__tests__/skypack.spec.ts b/packages/ruleset-bundler/src/plugins/__tests__/skypack.spec.ts
index 88e47e2c..f4cd9f83 100644
--- a/packages/ruleset-bundler/src/plugins/__tests__/skypack.spec.ts
+++ b/packages/ruleset-bundler/src/plugins/__tests__/skypack.spec.ts
@@ -11,12 +11,21 @@ import { skypack } from '../skypack';
 
 describe('Skypack Plugin', () => {
   let io: IO;
+  let warnSpy: jest.SpyInstance;
 
   beforeEach(() => {
     io = {
       fs,
       fetch,
     };
+
+    warnSpy = jest.spyOn(console, 'warn').mockImplementation(() => {
+      /* no-op */
+    });
+  });
+
+  afterEach(() => {
+    warnSpy.mockRestore();
   });
 
   describe.each<BundleOptions['target']>(['browser'])('given %s target', target => {