-
-
Notifications
You must be signed in to change notification settings - Fork 1.1k
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.
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
[WIP] Study possibility of moving from js-yaml to yaml #4135
Changes from all commits
e2886f0
07d18a3
357569e
969e02b
8cb4a60
707b19a
620a500
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -1,4 +1,8 @@ | ||
import {parseSyml} from '@yarnpkg/parsers'; | ||
import {parseSyml} from '@yarnpkg/parsers'; | ||
import fs from 'fs'; | ||
import { performance } from 'perf_hooks'; | ||
import {safeLoad as parseWithJsYaml, FAILSAFE_SCHEMA} from 'js-yaml'; | ||
import {parse as parseWithYaml } from 'yaml'; | ||
|
||
describe(`Syml parser`, () => { | ||
it(`shouldn't confuse old-style values with new-style keys`, () => { | ||
|
@@ -21,13 +25,48 @@ describe(`Syml parser`, () => { | |
|
||
it(`should merge duplicates`, () => { | ||
expect( | ||
parseSyml(` | ||
parseSyml(` | ||
"lodash@npm:^4.17.20": | ||
version: 4.17.20 | ||
|
||
"lodash@npm:^4.17.20": | ||
version: 4.17.20 | ||
`), | ||
).toEqual({'lodash@npm:^4.17.20': {version: `4.17.20`}}); | ||
}); | ||
}); | ||
|
||
describe('Yaml parse comparison (to be removed / reworked)', () => { | ||
it(`should parse identically as js-yaml@3`, () => { | ||
|
||
const lock = fs.readFileSync(`${__dirname}/../../../yarn.lock`, `utf8`); | ||
|
||
// js-yaml@3 doesn't parse `key: true` as a boolean, but as a string. | ||
// as far as I tested it makes the lock file invalid after install. | ||
const reviver = (key, value) => { | ||
if (value === true) { | ||
return 'true'; | ||
} else if (value === false) { | ||
return 'false'; | ||
} | ||
return value; | ||
} | ||
|
||
const startJsYaml = performance.now(); | ||
const jsYamlParsed = parseWithJsYaml(lock, {schema: FAILSAFE_SCHEMA}); | ||
const endJsYaml = performance.now(); | ||
const jsYamlTime = endJsYaml - startJsYaml; | ||
|
||
const startYaml = performance.now(); | ||
const yamlParsed = parseWithYaml(lock, reviver, { | ||
uniqueKeys: false, | ||
schema: 'failsafe', | ||
customTags: [] | ||
}) | ||
const endYaml = performance.now(); | ||
const yamlTime = endYaml - startYaml; | ||
|
||
expect(yamlParsed).toStrictEqual(jsYamlParsed); | ||
expect(yamlTime).toBeLessThan(jsYamlTime); | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Current finding: Expected: < 64.09353400021791 ms (js-yaml) Yes that might be a real blocker (at least for parsing yarn.lock which is not required). Adding both libs might not be a good move :( |
||
}) | ||
}) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
TODO: clear this, just for comparing the parsers on the yarn.lock