Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

fix(ruleset-bundler): never externalize builtins #2174

Merged
merged 3 commits into from Aug 3, 2022
Merged

fix(ruleset-bundler): never externalize builtins #2174

Show file tree
Hide file tree
Changes from 2 commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
77687fb
fix(ruleset-bundler): never externalize builtins
P0lip Jun 3, 2022
51fee55
test(ruleset-bundler): silent out external dep rollup warning
P0lip Jun 3, 2022
3db83fb
Merge branch 'develop' into fix/ruleset-bundler/always-inline-builtins
mnaumanali94 Aug 3, 2022
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
4 changes: 2 additions & 2 deletions packages/ruleset-bundler/package.json
View file
Open in desktop
Expand Up @@ -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",
Expand All @@ -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"
},
Expand Down
47 changes: 47 additions & 0 deletions packages/ruleset-bundler/src/__tests__/index.jest.test.ts
View file
Open in desktop
Expand Up @@ -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');

Expand Down Expand Up @@ -97,4 +98,50 @@ var spectral = {

export { spectral as default };`);
});

it('given node target, should support commonjs for remote ruleset with builtin modules', async () => {
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

taken from #2109

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',
},
},
},
},
},
};`);
});
});
60 changes: 1 addition & 59 deletions packages/ruleset-bundler/src/plugins/__tests__/builtins.spec.ts
View file
Open in desktop
Expand Up @@ -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';
Expand Down Expand Up @@ -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);
});
});
});
9 changes: 9 additions & 0 deletions packages/ruleset-bundler/src/plugins/__tests__/skypack.spec.ts
View file
Open in desktop
Expand Up @@ -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 => {
Expand Down
16 changes: 15 additions & 1 deletion packages/ruleset-bundler/src/plugins/builtins.ts
View file
Open in desktop
Expand Up @@ -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>;
Expand Down Expand Up @@ -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;
Expand Down
22 changes: 11 additions & 11 deletions yarn.lock
View file
Open in desktop
Expand Up @@ -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
Expand All @@ -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

Expand Down Expand Up @@ -2482,7 +2482,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"
Expand All @@ -2499,7 +2499,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
Expand Down Expand Up @@ -11193,17 +11193,17 @@ __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:
fsevents:
optional: true
bin:
rollup: dist/bin/rollup
checksum: 9aca5251ba4b441064183cde2394b91567259002d68086bdd3906db66d55dd148ab27e57c51eb53830d7b9b813c2d4e834b7735d65e2a869780bc639d4a20c38
checksum: fa3e61959efbc88cda75315dc22f035489be9dda3ae7d45a3a474f19efc3661ef6f9849486c7ca51e1a3e2aedef91f4e3223e8123bbeaf155913533d071994d1
languageName: node
linkType: hard

Expand Down