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.

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

Handle empty items in arrays and arraylike objects #75

Open
wants to merge 11 commits into
base: main
Choose a base branch
from
7 changes: 6 additions & 1 deletion lib/complexValues/object.js
Original file line number Diff line number Diff line change
Expand Up @@ -119,7 +119,12 @@ function DescribedMixin (base) {

const current = index
index++
return this.describeItem(current, this.describeAny(this.value[current]))

if (current in this.value) {
return this.describeItem(current, this.describeAny(this.value[current]))
} else {
return this.describeItem(current)
}
}

return { size, next }
Expand Down
4 changes: 4 additions & 0 deletions lib/describe.js
Original file line number Diff line number Diff line change
Expand Up @@ -133,6 +133,10 @@ function describeComplex (value, registry, tryPlugins, describeAny, describeItem
}

const describeItem = (index, valueDescriptor) => {
if (valueDescriptor === undefined) {
return itemDescriptor.describeEmpty(index)
}

return valueDescriptor.isPrimitive === true
? itemDescriptor.describePrimitive(index, valueDescriptor)
: itemDescriptor.describeComplex(index, valueDescriptor)
Expand Down
85 changes: 85 additions & 0 deletions lib/metaDescriptors/item.js
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@

const constants = require('../constants')
const formatUtils = require('../formatUtils')
const lineBuilder = require('../lineBuilder')
const recursorUtils = require('../recursorUtils')

const DEEP_EQUAL = constants.DEEP_EQUAL
Expand Down Expand Up @@ -30,12 +31,25 @@ function deserializePrimitive (state) {
}
exports.deserializePrimitive = deserializePrimitive

function describeEmpty (index) {
return new EmptyItem(index)
}
exports.describeEmpty = describeEmpty

function deserializeEmpty (index) {
return new EmptyItem(index)
}
exports.deserializeEmpty = deserializeEmpty

const complexTag = Symbol('ComplexItem')
exports.complexTag = complexTag

const primitiveTag = Symbol('PrimitiveItem')
exports.primitiveTag = primitiveTag

const emptyTag = Symbol('EmptyItem')
exports.emptyTag = emptyTag

class ComplexItem {
constructor (index, value) {
this.index = index
Expand Down Expand Up @@ -252,3 +266,74 @@ class PrimitiveItem {
}
Object.defineProperty(PrimitiveItem.prototype, 'isItem', { value: true })
Object.defineProperty(PrimitiveItem.prototype, 'tag', { value: primitiveTag })

class EmptyItem {
constructor (index) {
this.index = index
}

compare (expected) {
return expected.tag === emptyTag && this.index === expected.index
? DEEP_EQUAL
: UNEQUAL
}

formatDeep (theme, indent) {
const formatted = lineBuilder.single(formatUtils.wrap(theme.item.empty, 'empty item'))

if (typeof theme.item.customFormat === 'function') {
return theme.item.customFormat(theme, indent, formatted)
}

return formatted.withLastPostfixed(theme.item.after)
}

prepareDiff (expected, lhsRecursor, rhsRecursor, compareComplexShape, isCircular) {
const compareResult = this.compare(expected)
// Short-circuit when both are empty items.
if (compareResult === DEEP_EQUAL) return { compareResult }

// Try to line up this or remaining items with the expected items.
const rhsFork = recursorUtils.fork(rhsRecursor)
const initialExpected = expected

do {
if (expected === null || expected.isItem !== true) {
return {
actualIsExtraneous: true,
rhsRecursor: recursorUtils.map(
recursorUtils.unshift(rhsFork.recursor, initialExpected),
next => {
if (next.isItem !== true) return next

next.index++
return next
}),
}
}

if (this.compare(expected) === DEEP_EQUAL) {
return {
expectedIsMissing: true,
lhsRecursor: recursorUtils.map(
recursorUtils.unshift(lhsRecursor, this),
next => {
if (next.isItem !== true) return next

next.index++
return next
}),
rhsRecursor: rhsFork.recursor,
}
}

expected = rhsFork.shared()
} while (true)
}

serialize () {
return this.index
}
}
Object.defineProperty(EmptyItem.prototype, 'isItem', { value: true })
Object.defineProperty(EmptyItem.prototype, 'tag', { value: emptyTag })
4 changes: 3 additions & 1 deletion lib/serialize.js
Original file line number Diff line number Diff line change
Expand Up @@ -41,7 +41,7 @@ const recursorUtils = require('./recursorUtils')
// Concordance versions will not be able to decode buffers generated by a newer
// version, so changing this value will require a major version bump of
// Concordance itself. The version is encoded as an unsigned 16 bit integer.
const VERSION = 3
const VERSION = 4

// Adding or removing mappings or changing an index requires the version in
// encoder.js to be bumped, which necessitates a major version bump of
Expand Down Expand Up @@ -82,6 +82,8 @@ const mappings = [
[0x1D, setValue.tag, setValue.deserialize],
[0x1E, typedArrayValue.tag, typedArrayValue.deserialize],
[0x1F, typedArrayValue.bytesTag, typedArrayValue.deserializeBytes],

[0x20, itemDescriptor.emptyTag, itemDescriptor.deserializeEmpty],
]
const tag2id = new Map(mappings.map(mapping => [mapping[1], mapping[0]]))
const id2deserialize = new Map(mappings.map(mapping => [mapping[0], mapping[2]]))
Expand Down
1 change: 1 addition & 0 deletions lib/themeUtils.js
Original file line number Diff line number Diff line change
Expand Up @@ -47,6 +47,7 @@ const defaultTheme = freezeTheme({
item: {
after: ',',
customFormat: null,
empty: { open: '<', close: '>' },
increaseValueIndent: false,
},
list: { openBracket: '[', closeBracket: ']' },
Expand Down
8 changes: 8 additions & 0 deletions test/compare.js
Original file line number Diff line number Diff line change
Expand Up @@ -66,3 +66,11 @@ test('arrays are also compared by property', t => {
a4[-1] = -1
t.false(compare(a3, a4).pass)
})

test('empty array slots do not equal undefined', t => {
t.false(compare(new Array(1), Array.from({ length: 1 })).pass)
})

test('empty arraylike slots do not equal undefined', t => {
t.false(compare({ length: 2, 0: 'a' }, { length: 2, 0: 'a', 1: undefined }).pass)
})
Comment on lines +70 to +76
Copy link
Member

Choose a reason for hiding this comment

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

Perhaps this needs some tests for equality between arrays with empty items?

15 changes: 15 additions & 0 deletions test/diff.js
Original file line number Diff line number Diff line change
Expand Up @@ -566,3 +566,18 @@ test('diff pointers hidden behind maxDepth', t => {
concordance.diffDescriptors(concordance.deserialize(serialized), concordance.describe(undefined), { maxDepth: 1 })
})
})

test('diffs sparse arrays', t => {
t.snapshot(diff(new Array(3), Array.from({ length: 3 })))

const array = new Array(3)
array[1] = undefined
t.snapshot(diff(array, Array.from({ length: 3 })))
})

test('diffs sparse arraylike objects', t => {
t.snapshot(diff(
{ length: 3, 0: 'a', 2: 'c' },
{ length: 3, 0: 'a', 1: undefined, 2: 'c' },
))
})
Binary file modified test/fixtures/pointerSerialization.bin
Binary file not shown.
13 changes: 13 additions & 0 deletions test/format.js
Original file line number Diff line number Diff line change
Expand Up @@ -467,3 +467,16 @@ test('format pointers hidden behind maxDepth', t => {
concordance.formatDescriptor(concordance.deserialize(serialized), { maxDepth: 1 })
})
})

test('formats sparse arrays', t => {
const array1 = new Array(3)
t.snapshot(format(array1))

const array2 = new Array(3)
array2[1] = undefined
t.snapshot(format(array2))
})

test('formats sparse arraylike objects', t => {
t.snapshot(format({ length: 4, 0: 'foo', 2: 'bar' }))
})
4 changes: 2 additions & 2 deletions test/lodash-isequal-comparison.js
Original file line number Diff line number Diff line change
Expand Up @@ -82,7 +82,7 @@ test('compare arrays', t => {
array2[1] = undefined
array2[2] = 3

t.true(isEqual(array1, array2))
t.false(isEqual(array1, array2))

array1 = [new Object(1), false, new Object('a'), /x/, new Date(2012, 4, 23), ['a', 'b', [new Object('c')]], { a: 1 }]
array2 = [1, new Object(false), 'a', /x/, new Date(2012, 4, 23), ['a', new Object('b'), ['c']], { a: 1 }]
Expand Down Expand Up @@ -132,7 +132,7 @@ test('compare sparse arrays', t => {
const array = new Array(1)

t.true(isEqual(array, new Array(1)))
t.true(isEqual(array, [undefined]))
t.false(isEqual(array, [undefined]))
t.false(isEqual(array, new Array(2)))
})

Expand Down
1 change: 1 addition & 0 deletions test/serialize-and-encode.js
Original file line number Diff line number Diff line change
Expand Up @@ -119,6 +119,7 @@ test('symbol properties are reordered despite serialization', t => {
// Arrays
test('array with primitive item', useDeserialized, ['bar'])
test('array with complex item', useDeserialized, [{}])
test('array with empty item', useDeserialized, new Array(1))

// Iterators
test('iterator with primitive item', useDeserialized,
Expand Down
36 changes: 36 additions & 0 deletions test/snapshots/diff.js.md
Original file line number Diff line number Diff line change
Expand Up @@ -1253,3 +1253,39 @@ Generated by [AVA](https://avajs.dev).
%diffGutters.actual#- % e%property.separator#: %%object.ctor.open%Object%object.ctor.close% %object.openBracket#{% %maxDepth#…% %object.closeBracket#}%%property.after#,%␊
%diffGutters.actual#- % %object.closeBracket#}%%property.after#,%␊
%diffGutters.padding# %%object.closeBracket#}%`

## diffs sparse arrays

> Snapshot 1

`%diffGutters.padding# %%list.openBracket#[%␊
%diffGutters.actual#- % %item.empty.open#<%empty item%item.empty.close#>%%item.after#,%␊
%diffGutters.actual#- % %item.empty.open#<%empty item%item.empty.close#>%%item.after#,%␊
%diffGutters.actual#- % %item.empty.open#<%empty item%item.empty.close#>%%item.after#,%␊
%diffGutters.expected#+ % %undefined.open%undefined%undefined.close%%item.after#,%␊
%diffGutters.expected#+ % %undefined.open%undefined%undefined.close%%item.after#,%␊
%diffGutters.expected#+ % %undefined.open%undefined%undefined.close%%item.after#,%␊
%diffGutters.padding# %%list.closeBracket#]%`

> Snapshot 2

`%diffGutters.padding# %%list.openBracket#[%␊
%diffGutters.actual#- % %item.empty.open#<%empty item%item.empty.close#>%%item.after#,%␊
%diffGutters.padding# % %undefined.open%undefined%undefined.close%%item.after#,%␊
%diffGutters.actual#- % %item.empty.open#<%empty item%item.empty.close#>%%item.after#,%␊
%diffGutters.expected#+ % %undefined.open%undefined%undefined.close%%item.after#,%␊
%diffGutters.expected#+ % %undefined.open%undefined%undefined.close%%item.after#,%␊
%diffGutters.padding# %%list.closeBracket#]%`

## diffs sparse arraylike objects

> Snapshot 1

`%diffGutters.padding# %%list.openBracket#[%␊
%diffGutters.padding# % %string.line.open#'%%string.open%a%string.close%%string.line.close#'%%item.after#,%␊
%diffGutters.actual#- % %item.empty.open#<%empty item%item.empty.close#>%%item.after#,%␊
%diffGutters.expected#+ % %undefined.open%undefined%undefined.close%%item.after#,%␊
%diffGutters.padding# % %string.line.open#'%%string.open%c%string.close%%string.line.close#'%%item.after#,%␊
%diffGutters.padding# % %stats.separator#---%␊
%diffGutters.padding# % length%property.separator#: %%number.open%3%number.close%%property.after#,%␊
%diffGutters.padding# %%list.closeBracket#]%`
Binary file modified test/snapshots/diff.js.snap
Binary file not shown.
31 changes: 31 additions & 0 deletions test/snapshots/format.js.md
Original file line number Diff line number Diff line change
Expand Up @@ -904,3 +904,34 @@ Generated by [AVA](https://avajs.dev).
`Error {␊
message: 'error',␊
}`

## formats sparse arrays

> Snapshot 1

`%list.openBracket#[%␊
%item.empty.open#<%empty item%item.empty.close#>%%item.after#,%␊
%item.empty.open#<%empty item%item.empty.close#>%%item.after#,%␊
%item.empty.open#<%empty item%item.empty.close#>%%item.after#,%␊
%list.closeBracket#]%`

> Snapshot 2

`%list.openBracket#[%␊
%item.empty.open#<%empty item%item.empty.close#>%%item.after#,%␊
%undefined.open%undefined%undefined.close%%item.after#,%␊
%item.empty.open#<%empty item%item.empty.close#>%%item.after#,%␊
%list.closeBracket#]%`

## formats sparse arraylike objects

> Snapshot 1

`%list.openBracket#[%␊
%string.line.open#'%%string.open%foo%string.close%%string.line.close#'%%item.after#,%␊
%item.empty.open#<%empty item%item.empty.close#>%%item.after#,%␊
%string.line.open#'%%string.open%bar%string.close%%string.line.close#'%%item.after#,%␊
%item.empty.open#<%empty item%item.empty.close#>%%item.after#,%␊
%stats.separator#---%␊
length%property.separator#: %%number.open%4%number.close%%property.after#,%␊
%list.closeBracket#]%`
Binary file modified test/snapshots/format.js.snap
Binary file not shown.