Skip to content

Commit 4369134

Browse files
authored
refactor: remove yaml dependency, use js-yaml instead. (#4134)
Problem: 'yaml' is outdated and requires human intervention to migrate to V2. It is only used in a few places in the codebase, whereas everywhere else, a similar yaml parser js-yaml is used. Solution: remove usage of 'yaml' and only use 'js-yaml', as it has some features that we need (ability to write out to a string and skip invalid JSON entries, which 'yaml' does not seem to offer). We do not require all the features of 'yaml' at this time. Also, remove "errors" field from the state machines parser -> webview message. It is not used by anything and was only accessible with 'yaml'. In one case of 'yaml', we used the 'maxAliasCount' option to prevent users from submitting state machines with aliases in them. Aliases do not prevent publishing normally to step functions if used normally; I believe it is to prevent the user from breaking the toolkit by exponentially copying anchors/aliases in the yaml. This does not affect publishing (it will fail as it is supposed to if the defintion is too large), and slightly slows down toolkit usage while the parse is happening. This validation is not necessary because this sort of behavior would be very specific and intentional from the user, and in my testing still causes slow downs and errors due to the logic centered around how the validation option was used.
1 parent 4800116 commit 4369134

File tree

6 files changed

+44
-34
lines changed

6 files changed

+44
-34
lines changed

package-lock.json

Lines changed: 0 additions & 1 deletion
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

package.json

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -4399,7 +4399,6 @@
43994399
"winston": "^3.7.1",
44004400
"winston-transport": "^4.5.0",
44014401
"xml2js": "^0.6.0",
4402-
"yaml": "^1.9.2",
44034402
"yaml-cfn": "^0.3.2"
44044403
},
44054404
"prettier": {

src/ssmDocument/commands/createDocumentFromTemplate.ts

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -8,7 +8,7 @@ const localize = nls.loadMessageBundle()
88

99
import * as path from 'path'
1010
import * as vscode from 'vscode'
11-
import * as YAML from 'yaml'
11+
import * as yaml from 'js-yaml'
1212
import { getLogger, Logger } from '../../shared/logger'
1313
import * as picker from '../../shared/ui/picker'
1414
import { promptUserForDocumentFormat } from '../util/util'
@@ -114,9 +114,9 @@ export async function createSsmDocumentFromTemplate(extensionContext: vscode.Ext
114114
}
115115
}
116116

117-
function yamlToJson(yaml: string): string {
118-
const jsonObject = YAML.parse(yaml)
119-
return JSON.stringify(jsonObject, undefined, '\t')
117+
function yamlToJson(yamlStr: string): string {
118+
const jsonObject = yaml.load(yamlStr, { schema: yaml.JSON_SCHEMA })
119+
return JSON.stringify(jsonObject, undefined, 4)
120120
}
121121

122122
async function openTextDocumentFromSelection(

src/stepFunctions/commands/visualizeStateMachine/aslVisualization.ts

Lines changed: 6 additions & 17 deletions
Original file line numberDiff line numberDiff line change
@@ -10,19 +10,11 @@ import * as path from 'path'
1010
import * as vscode from 'vscode'
1111
import { getLogger, Logger } from '../../../shared/logger'
1212
import { isDocumentValid } from '../../utils'
13-
import * as yaml from 'yaml'
13+
import * as yaml from 'js-yaml'
1414

1515
import { YAML_FORMATS } from '../../constants/aslFormats'
1616
import globals from '../../../shared/extensionGlobals'
1717

18-
const yamlOptions: yaml.Options = {
19-
merge: false,
20-
maxAliasCount: 0,
21-
schema: 'yaml-1.1',
22-
version: '1.1',
23-
prettyErrors: true,
24-
}
25-
2618
export interface MessageObject {
2719
command: string
2820
text: string
@@ -67,23 +59,21 @@ export class AslVisualization {
6759
const isYaml = YAML_FORMATS.includes(updatedTextDocument.languageId)
6860
const text = this.getText(updatedTextDocument)
6961
let stateMachineData = text
70-
let yamlErrors: string[] = []
7162

7263
if (isYaml) {
73-
const parsed = yaml.parseDocument(text, yamlOptions)
74-
yamlErrors = parsed.errors.map(error => error.message)
7564
let json: any
7665

7766
try {
78-
json = parsed.toJSON()
79-
} catch (e) {
80-
yamlErrors.push((e as Error).message)
67+
json = yaml.load(text, { schema: yaml.JSON_SCHEMA })
68+
} catch (e: any) {
69+
const msg = e instanceof yaml.YAMLException ? e.message : 'unknown error'
70+
getLogger().error(`Error parsing state machine YAML: ${msg}`)
8171
}
8272

8373
stateMachineData = JSON.stringify(json)
8474
}
8575

86-
const isValid = (await isDocumentValid(stateMachineData, updatedTextDocument)) && !yamlErrors.length
76+
const isValid = await isDocumentValid(stateMachineData, updatedTextDocument)
8777

8878
const webview = this.getWebview()
8979
if (this.isPanelDisposed || !webview) {
@@ -96,7 +86,6 @@ export class AslVisualization {
9686
command: 'update',
9787
stateMachineData,
9888
isValid,
99-
errors: yamlErrors,
10089
})
10190
}
10291

src/test/ssmDocument/commands/createDocumentFromTemplate.test.ts

Lines changed: 11 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -14,7 +14,6 @@ import {
1414
import * as ssmDocumentUtil from '../../../ssmDocument/util/util'
1515
import * as fsUtilities from '../../../shared/filesystemUtilities'
1616

17-
import YAML from 'yaml'
1817
import { FakeExtensionContext } from '../../fakeExtensionContext'
1918

2019
describe('createDocumentFromTemplate', async function () {
@@ -31,10 +30,15 @@ describe('createDocumentFromTemplate', async function () {
3130
sandbox.restore()
3231
})
3332

34-
const fakeContent = `{
35-
"schemaVersion": "2.2",
36-
"mainSteps": []
37-
}`
33+
const fakeContentYaml = `---
34+
schemaVersion: '2.2'
35+
mainSteps: []
36+
`
37+
38+
const fakeContentJson = `{
39+
"schemaVersion": "2.2",
40+
"mainSteps": []
41+
}`
3842

3943
const fakeSelectionResult: SsmDocumentTemplateQuickPickItem = {
4044
label: 'test template',
@@ -49,16 +53,14 @@ describe('createDocumentFromTemplate', async function () {
4953
it('open and save document based on selected template', async function () {
5054
sandbox.stub(picker, 'promptUser').returns(Promise.resolve(fakeSelection))
5155
sandbox.stub(picker, 'verifySinglePickerOutput').returns(fakeSelectionResult)
52-
sandbox.stub(fsUtilities, 'readFileAsString').returns(Promise.resolve(fakeContent))
56+
sandbox.stub(fsUtilities, 'readFileAsString').returns(Promise.resolve(fakeContentYaml))
5357
sandbox.stub(ssmDocumentUtil, 'promptUserForDocumentFormat').returns(Promise.resolve('JSON'))
54-
sandbox.stub(JSON, 'stringify').returns(fakeContent)
55-
sandbox.stub(YAML, 'stringify').returns(fakeContent)
5658

5759
const openTextDocumentStub = sandbox.stub(vscode.workspace, 'openTextDocument')
5860

5961
await createSsmDocumentFromTemplate(mockContext)
6062

61-
assert.strictEqual(openTextDocumentStub.getCall(0).args[0]?.content, fakeContent)
63+
assert.strictEqual(openTextDocumentStub.getCall(0).args[0]?.content, fakeContentJson)
6264
assert.strictEqual(openTextDocumentStub.getCall(0).args[0]?.language, 'ssm-json')
6365
})
6466
})

src/test/stepFunctions/commands/visualizeStateMachine.test.ts

Lines changed: 23 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -222,7 +222,29 @@ describe('StepFunctions VisualizeStateMachine', async function () {
222222
command: 'update',
223223
stateMachineData: mockDataJson,
224224
isValid: true,
225-
errors: [],
225+
}
226+
227+
assert.ok(postMessage.calledOnce)
228+
assert.deepEqual(postMessage.firstCall.args, [message])
229+
})
230+
231+
it('Test AslVisualisation sendUpdateMessage posts a correct update message for invalid YAML files', async function () {
232+
const yamlDoc = await getDocument(mockDataYaml.replace('StartAt:', ']StartAt:'), YAML_ASL)
233+
const postMessage = sinon.spy()
234+
class MockAslVisualizationYaml extends AslVisualization {
235+
public override getWebview(): vscode.Webview | undefined {
236+
return { postMessage } as unknown as vscode.Webview
237+
}
238+
}
239+
240+
const visualisation = new MockAslVisualizationYaml(yamlDoc)
241+
242+
await visualisation.sendUpdateMessage(yamlDoc)
243+
244+
const message = {
245+
command: 'update',
246+
stateMachineData: undefined,
247+
isValid: false,
226248
}
227249

228250
assert.ok(postMessage.calledOnce)
@@ -246,7 +268,6 @@ describe('StepFunctions VisualizeStateMachine', async function () {
246268
command: 'update',
247269
stateMachineData: mockDataJson,
248270
isValid: true,
249-
errors: [],
250271
}
251272

252273
assert.ok(postMessage.calledOnce)

0 commit comments

Comments
 (0)