Open tommy-mitchell opened 1 year ago
tommy-mitchell
commented
1 year ago Perhaps we could use smoldash instead of lodash for _.cloneDeep and _.merge: https://github.com/marvinhagemeister/smoldash/blob/51b5872d8c0887e25e0ccd54555b35f0d7ecb2a8/src/index.ts#L286-L339
Related, what is _.isLength doing? Is it needed?
novemberborn
commented
1 year ago
- Updates dependencies (except for
well-known-symbols)
That library has transferred actually (not sure if it's been republished yet). There's definitely more well-known symbols out there but maybe there's a better way to encode them.
Perhaps we could use
smoldashinstead oflodashfor_.cloneDeepand_.merge: marvinhagemeister/smoldash@51b5872/src/index.ts#L286-L339
I'm not bothered either way. There may also be some APIs we could use, e.g. the structured clone implementation in the node:v8 module.
Related, what is
_.isLengthdoing? Is it needed?
Looks like it's a heuristic for identifying when to treat an object as a list (array). But it could be inlined:
- Bumps minimum Node.js version to v16.9 accordingly (MDN compatibility list) and updates CI matrix
I'd go with 18. 16 is end of life in 9 days.
tommy-mitchell
commented
3 months ago Updated to bump minimum Node.js version and remove Lodash in favor of inlining _.isLength and using spread and structuredClone over _.merge and _.cloneDeep. Also updates well-known-symbols to v4, which changes how well-known symbols are formatted:
- Symbol.iterator
+ Symbol(Symbol.iterator)Still need to fix the failing tests. One issue seems to be how undefined is handled between spread and _.merge, as mentioned in #78
```
serialization-fixtures › pointer serialization equals the same tree
test/serialization-fixtures.js:11
10: t.log(diff(deserialize(serialization), describe(tree)));
11: t.true(compareDescriptors(deserialize(serialization), describe(tree)));
12: });
Value is not `true`:
false
› file://test/serialization-fixtures.js:11:8
max-depth › properties with increased indentation respect the maxDepth when formatted
test/max-depth.js:45
44: test('properties with increased indentation respect the maxDepth when fo…
45: t.snapshot(format({
46: foo: {
Did not match snapshot
Difference (- actual, + expected):
`{␊
- fooundefined…undefined␊
+ foo: …,␊
}`
› file://test/max-depth.js:45:4
diff › diff pointers hidden behind maxDepth
test/diff.js:594
593:
594: t.notThrows(() => {
595: // `maxDepth: 1` means that `a.b` is not normally deserialized, and…
Function threw:
Error {
message: 'Formatter buffer never received a formatted value.',
}
› SingleValueFormatter.finalize (file://lib/formatUtils.js:119:10)
› format (file://lib/diff.js:181:38)
› Object.diffDescriptors (file://lib/diff.js:343:6)
› file://test/diff.js:598:15
› file://test/diff.js:594:4
format › escapes ' according to theme
test/format.js:93
92: };
93: t.snapshot(_format(escapeQuote, {theme: testTheme}));
94: t.snapshot(_format(escapeQuote + '\n', {theme: testTheme}));
Did not match snapshot
Difference (- actual, + expected):
- '
tommy-mitchell
commented
3 months ago Updated to use the defaults package to handle merging theme configs.
``` serialization-fixtures › pointer serialization equals the same tree test/serialization-fixtures.js:11 10: t.log(diff(deserialize(serialization), describe(tree))); 11: t.true(compareDescriptors(deserialize(serialization), describe(tree))); 12: }); Value is not `true`: false › file://test/serialization-fixtures.js:11:8 diff › diff pointers hidden behind maxDepth test/diff.js:594 593: 594: t.notThrows(() => { 595: // `maxDepth: 1` means that `a.b` is not normally deserialized, and… Function threw: Error { message: 'Formatter buffer never received a formatted value.', } › SingleValueFormatter.finalize (file://lib/formatUtils.js:119:10) › format (file://lib/diff.js:181:38) › Object.diffDescriptors (file://lib/diff.js:343:6) › file://test/diff.js:598:15 › file://test/diff.js:594:4 format › format pointers hidden behind maxDepth test/format.js:477 476: 477: t.notThrows(() => { 478: // `maxDepth: 1` means that `a.b` is not normally deserialized, and… Function threw: Error { message: 'Formatter buffer never received a formatted value.', } › SingleValueFormatter.finalize (file://lib/formatUtils.js:119:10) › Object.formatDescriptor (file://lib/format.js:92:39) › file://test/format.js:481:15 › file://test/format.js:477:4 serialize-and-encode › deserialized object with pointer to itself is equivalent to the original test/serialize-and-encode.js:42 41: 42: t.true( 43: compareDescriptors(deserialized, original), the deserialized descriptor equals the original Value is not `true`: false › exec (file://test/serialize-and-encode.js:42:9) serialize-and-encode › deserialized plugin is equivalent to the original test/serialize-and-encode.js:42 41: 42: t.true( 43: compareDescriptors(deserialized, original), the deserialized descriptor equals the original Value is not `true`: false › exec (file://test/serialize-and-encode.js:42:9) ─ 5 tests failed ```
tommy-mitchell
commented
3 months ago Maybe the well-known-symbols update should be done in a separate PR? Since it would be a breaking change (as mentioned in #59). It could be nice to release a refactor update, and then follow with a breaking change update.
Per #82.
"engines"version range fromAVAObject.hasOwn()instead ofObject.prototype.hasOwnProperty.call()xo(based on configs fromAVA)well-known-symbolsshim update changes how well-known symbols are formatted (Symbol.iterator->Symbol(Symbol.iterator))A few tests are failing, seems to have to do with serialization and object pointers.