Skip to content

Commit

Permalink
Merge pull request #1241 from RoadieHQ/sc-18744-globalization-partner…
Browse files Browse the repository at this point in the history
…s-swap-yaml-library

Swap YAML library in Scaffolder actions to a better one
  • Loading branch information
JoaoMartins51 authored Feb 7, 2024
2 parents 500ec31 + 42db6b8 commit e8d1a74
Show file tree
Hide file tree
Showing 14 changed files with 59 additions and 48 deletions.
5 changes: 5 additions & 0 deletions .changeset/good-cars-happen.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
---
'@roadiehq/scaffolder-backend-module-utils': patch
---

Swapping 'js-yaml' library for 'yaml'
Original file line number Diff line number Diff line change
Expand Up @@ -40,7 +40,7 @@
"cross-fetch": "^3.1.4",
"detect-indent": "^6.1.0",
"fs-extra": "^10.0.0",
"js-yaml": "^4.0.0",
"yaml": "^2.3.4",
"jsonata": "^1.8.6",
"lodash": "^4.17.21",
"winston": "^3.2.1"
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -42,7 +42,7 @@ describe('roadiehq:utils:fs:append', () => {
...mockContext,
input: { content: '' } as any,
}),
).rejects.toThrow(/"path" argument must/);
).rejects.toThrow(/argument must be of type string/);
});
it('should write file to the workspacePath with the given content if it doesnt exist', async () => {
await action.handler({
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -40,7 +40,7 @@ describe('roadiehq:utils:fs:parse', () => {
...mockContext,
input: {} as any,
}),
).rejects.toThrow(/"path" argument must/);
).rejects.toThrow(/argument must be of type string/);
});
it('should pasrs text data by default', async () => {
mock({
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -16,12 +16,13 @@
import { createTemplateAction } from '@backstage/plugin-scaffolder-backend';
import { resolveSafeChildPath } from '@backstage/backend-common';
import fs from 'fs-extra';
import yaml from 'js-yaml';
import YAML from 'yaml';

const parsers: Record<'yaml' | 'json' | 'multiyaml', (cnt: string) => any> = {
yaml: (cnt: string) => yaml.load(cnt),
yaml: (cnt: string) => YAML.parse(cnt),
json: (cnt: string) => JSON.parse(cnt),
multiyaml: (cnt: string) => yaml.loadAll(cnt),
multiyaml: (cnt: string) =>
YAML.parseAllDocuments(cnt).map(doc => doc.toJSON()),
};

export function createParseFileAction() {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -41,7 +41,7 @@ describe('roadiehq:utils:fs:write', () => {
...mockContext,
input: {} as any,
}),
).rejects.toThrow(/"path" argument must/);
).rejects.toThrow(/argument must be of type string/);
});
it('should write file to the workspacePath with the given content', async () => {
await action.handler({
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -18,7 +18,7 @@ import { getVoidLogger } from '@backstage/backend-common';
import { createYamlJSONataTransformAction } from './yaml';
import { PassThrough } from 'stream';
import mock from 'mock-fs';
import yaml from 'js-yaml';
import YAML from 'yaml';

describe('roadiehq:utils:jsonata:yaml:transform', () => {
const mockContext = {
Expand Down Expand Up @@ -56,7 +56,7 @@ describe('roadiehq:utils:jsonata:yaml:transform', () => {

expect(mockContext.output).toHaveBeenCalledWith(
'result',
yaml.dump({ hello: ['world', 'item2'] }),
YAML.stringify({ hello: ['world', 'item2'] }),
);
});

Expand All @@ -78,18 +78,18 @@ describe('roadiehq:utils:jsonata:yaml:transform', () => {

expect(mockContext.output).toHaveBeenCalledWith(
'result',
yaml.dump({ hello: ['world', 'item2'] }),
YAML.stringify({ hello: ['world', 'item2'] }),
);
});

it('should pass options to yaml.dump', async () => {
it('should pass options to YAML.stringify', async () => {
mock({
'fake-tmp-dir': {
'fake-file.yaml': '',
},
});

const mockDump = jest.spyOn(yaml, 'dump');
const mockDump = jest.spyOn(YAML, 'stringify');

const opts = {
indent: 3,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -17,14 +17,14 @@ import { createTemplateAction } from '@backstage/plugin-scaffolder-backend';
import jsonata from 'jsonata';
import { resolveSafeChildPath } from '@backstage/backend-common';
import fs from 'fs-extra';
import yaml from 'js-yaml';
import { supportedDumpOptions, yamlOptionsSchema } from '../../types';
import YAML from 'yaml';
import { stringifyOptions, yamlOptionsSchema } from '../../types';

export function createYamlJSONataTransformAction() {
return createTemplateAction<{
path: string;
expression: string;
options?: supportedDumpOptions;
options?: stringifyOptions;
loadAll?: boolean;
as?: 'string' | 'object';
}>({
Expand Down Expand Up @@ -79,17 +79,19 @@ export function createYamlJSONataTransformAction() {
if (ctx.input.as === 'object') {
resultHandler = rz => rz;
} else {
resultHandler = rz => yaml.dump(rz, ctx.input.options);
resultHandler = rz => YAML.stringify(rz, ctx.input.options);
}
const sourceFilepath = resolveSafeChildPath(
ctx.workspacePath,
ctx.input.path,
);
let data;
if (ctx.input.loadAll) {
data = yaml.loadAll(fs.readFileSync(sourceFilepath).toString());
data = YAML.parseAllDocuments(
fs.readFileSync(sourceFilepath).toString(),
).map(doc => doc.toJSON());
} else {
data = yaml.load(fs.readFileSync(sourceFilepath).toString());
data = YAML.parse(fs.readFileSync(sourceFilepath).toString());
}
const expression = jsonata(ctx.input.expression);
const result = expression.evaluate(data);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -19,7 +19,7 @@ import { createMergeAction, createMergeJSONAction } from './merge';
import { PassThrough } from 'stream';
import mock from 'mock-fs';
import fs from 'fs-extra';
import yaml from 'js-yaml';
import YAML from 'yaml';
import detectIndent from 'detect-indent';

describe('roadiehq:utils:json:merge', () => {
Expand All @@ -45,7 +45,7 @@ describe('roadiehq:utils:json:merge', () => {
...mockContext,
input: { content: '' } as any,
}),
).rejects.toThrow(/"path" argument must/);
).rejects.toThrow(/argument must be of type string/);
});

it('should throw an error when the source file is not json', async () => {
Expand Down Expand Up @@ -297,7 +297,7 @@ describe('roadiehq:utils:merge', () => {
...mockContext,
input: { content: '' } as any,
}),
).rejects.toThrow(/"path" argument must/);
).rejects.toThrow(/argument must be of type string/);
});

it('should throw an error when the source file does not exist', async () => {
Expand Down Expand Up @@ -388,7 +388,7 @@ scripts:

expect(fs.existsSync('fake-tmp-dir/fake-file.yaml')).toBe(true);
const file = fs.readFileSync('fake-tmp-dir/fake-file.yaml', 'utf-8');
expect(yaml.load(file)).toEqual({
expect(YAML.parse(file)).toEqual({
scripts: { lsltr: 'ls -ltr', lsltrh: 'ls -ltrh' },
});
});
Expand Down Expand Up @@ -416,7 +416,7 @@ scripts:

expect(fs.existsSync('fake-tmp-dir/fake-file.yaml')).toBe(true);
const file = fs.readFileSync('fake-tmp-dir/fake-file.yaml', 'utf-8');
expect(yaml.load(file)).toEqual({
expect(YAML.parse(file)).toEqual({
scripts: { lsltr: 'ls -ltr', lsltrh: 'ls -ltrh' },
array: ['second item'],
});
Expand All @@ -443,19 +443,19 @@ scripts:

expect(fs.existsSync('fake-tmp-dir/fake-file.yaml')).toBe(true);
const file = fs.readFileSync('fake-tmp-dir/fake-file.yaml', 'utf-8');
expect(yaml.load(file)).toEqual({
expect(YAML.parse(file)).toEqual({
array: ['first item', 'second item'],
});
});

it('can pass options to yaml.dump', async () => {
it('can pass options to YAML.stringify', async () => {
mock({
'fake-tmp-dir': {
'fake-file.yaml': '',
},
});

const mockDump = jest.spyOn(yaml, 'dump');
const mockDump = jest.spyOn(YAML, 'stringify');

const opts = {
indent: 3,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -19,8 +19,8 @@ import { resolveSafeChildPath } from '@backstage/backend-common';
import fs from 'fs-extra';
import { extname } from 'path';
import { isArray, isNull, mergeWith } from 'lodash';
import yaml from 'js-yaml';
import { supportedDumpOptions, yamlOptionsSchema } from '../../types';
import YAML from 'yaml';
import { stringifyOptions, yamlOptionsSchema } from '../../types';
import detectIndent from 'detect-indent';

function mergeArrayCustomiser(objValue: string | any[], srcValue: any) {
Expand Down Expand Up @@ -139,7 +139,7 @@ export function createMergeAction() {
path: string;
content: any;
mergeArrays?: boolean;
options?: supportedDumpOptions;
options?: stringifyOptions;
}>({
id: 'roadiehq:utils:merge',
description: 'Merges data into an existing structured file.',
Expand Down Expand Up @@ -204,7 +204,7 @@ export function createMergeAction() {
: ctx.input.content; // This supports the case where dynamic keys are required
mergedContent = JSON.stringify(
mergeWith(
yaml.load(originalContent),
YAML.parse(originalContent),
newContent,
ctx.input.mergeArrays ? mergeArrayCustomiser : undefined,
),
Expand All @@ -217,11 +217,11 @@ export function createMergeAction() {
case '.yaml': {
const newContent =
typeof ctx.input.content === 'string'
? yaml.load(ctx.input.content)
? YAML.parse(ctx.input.content)
: ctx.input.content; // This supports the case where dynamic keys are required
mergedContent = yaml.dump(
mergedContent = YAML.stringify(
mergeWith(
yaml.load(originalContent),
YAML.parse(originalContent),
newContent,
ctx.input.mergeArrays ? mergeArrayCustomiser : undefined,
),
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -16,7 +16,7 @@
import { getVoidLogger } from '@backstage/backend-common';
import { createSerializeYamlAction } from './yaml';
import { PassThrough } from 'stream';
import yaml from 'js-yaml';
import YAML from 'yaml';

describe('roadiehq:utils:serialize:yaml', () => {
const mockContext = {
Expand All @@ -39,7 +39,7 @@ describe('roadiehq:utils:serialize:yaml', () => {

expect(mockContext.output).toHaveBeenCalledWith(
'serialized',
yaml.dump({ hello: 'world' }),
YAML.stringify({ hello: 'world' }),
);
});

Expand All @@ -57,12 +57,15 @@ describe('roadiehq:utils:serialize:yaml', () => {

expect(mockContext.output).toHaveBeenCalledWith(
'serialized',
yaml.dump({ hello1: 'world1', hello2: { asdf: 'blah' } }, { indent: 10 }),
YAML.stringify(
{ hello1: 'world1', hello2: { asdf: 'blah' } },
{ indent: 10 },
),
);
});

it('should pass options to yaml.dump', async () => {
const mockDump = jest.spyOn(yaml, 'dump');
it('should pass options to YAML.stringify', async () => {
const mockDump = jest.spyOn(YAML, 'stringify');

const opts = {
indent: 3,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -14,13 +14,13 @@
* limitations under the License.
*/
import { createTemplateAction } from '@backstage/plugin-scaffolder-node';
import yaml from 'js-yaml';
import { supportedDumpOptions, yamlOptionsSchema } from '../../types';
import YAML from 'yaml';
import { stringifyOptions, yamlOptionsSchema } from '../../types';

export function createSerializeYamlAction() {
return createTemplateAction<{
data: any;
options?: supportedDumpOptions;
options?: stringifyOptions;
}>({
id: 'roadiehq:utils:serialize:yaml',
description: 'Allows performing serialization on an object',
Expand Down Expand Up @@ -58,7 +58,10 @@ export function createSerializeYamlAction() {
},

async handler(ctx) {
ctx.output('serialized', yaml.dump(ctx.input.data, ctx.input.options));
ctx.output(
'serialized',
YAML.stringify(ctx.input.data, ctx.input.options),
);
},
});
}
Original file line number Diff line number Diff line change
Expand Up @@ -53,7 +53,7 @@ describe('roadiehq:utils:zip', () => {
...mockContext,
input: {} as any,
}),
).rejects.toThrow(/"path" argument must/);
).rejects.toThrow(/argument must be of type string/);
});
it("should create a zip file called 'outputPath' from the given 'path'", async () => {
await action.handler({
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -14,12 +14,9 @@
* limitations under the License.
*/

import { DumpOptions } from 'js-yaml';
import { ToStringOptions } from 'yaml';

export type supportedDumpOptions = Omit<
DumpOptions,
'styles' | 'schema' | 'replacer' | 'sortKeys'
>;
export type stringifyOptions = Omit<ToStringOptions, 'commentString'>;

export const yamlOptionsSchema = {
title: 'Options',
Expand Down

0 comments on commit e8d1a74

Please sign in to comment.