Skip to content

Commit

Permalink
Refactor: restore use of spread operator (#106)
Browse files Browse the repository at this point in the history
Because the Chromium bug has been fixed.

Ref: https://bugs.chromium.org/p/chromium/issues/detail?id=1204540
Category: fix
  • Loading branch information
vweevers authored Dec 31, 2024
1 parent a05a8ea commit a5c2e52
Show file tree
Hide file tree
Showing 4 changed files with 33 additions and 33 deletions.
20 changes: 10 additions & 10 deletions abstract-chained-batch.js
Original file line number Diff line number Diff line change
Expand Up @@ -80,14 +80,14 @@ class AbstractChainedBatch {
if (keyError != null) throw keyError
if (valueError != null) throw valueError

// Avoid spread operator because of https://bugs.chromium.org/p/chromium/issues/detail?id=1204540
const op = Object.assign({}, options, {
const op = {
...options,
type: 'put',
key,
value,
keyEncoding: db.keyEncoding(options.keyEncoding),
valueEncoding: db.valueEncoding(options.valueEncoding)
})
}

if (this.#prewriteRun !== null) {
try {
Expand Down Expand Up @@ -135,7 +135,7 @@ class AbstractChainedBatch {
// If the sublevel is not a descendant then we shouldn't emit events
if (this.#publicOperations !== null && !siblings) {
// Clone op before we mutate it for the private API
const publicOperation = Object.assign({}, op)
const publicOperation = { ...op }
publicOperation.encodedKey = encodedKey
publicOperation.encodedValue = encodedValue

Expand All @@ -149,7 +149,7 @@ class AbstractChainedBatch {

this.#publicOperations.push(publicOperation)
} else if (this.#legacyOperations !== null && !siblings) {
const legacyOperation = Object.assign({}, original)
const legacyOperation = { ...original }

legacyOperation.type = 'put'
legacyOperation.key = key
Expand Down Expand Up @@ -189,12 +189,12 @@ class AbstractChainedBatch {

if (keyError != null) throw keyError

// Avoid spread operator because of https://bugs.chromium.org/p/chromium/issues/detail?id=1204540
const op = Object.assign({}, options, {
const op = {
...options,
type: 'del',
key,
keyEncoding: db.keyEncoding(options.keyEncoding)
})
}

if (this.#prewriteRun !== null) {
try {
Expand All @@ -221,7 +221,7 @@ class AbstractChainedBatch {

if (this.#publicOperations !== null) {
// Clone op before we mutate it for the private API
const publicOperation = Object.assign({}, op)
const publicOperation = { ...op }
publicOperation.encodedKey = encodedKey

if (delegated) {
Expand All @@ -232,7 +232,7 @@ class AbstractChainedBatch {

this.#publicOperations.push(publicOperation)
} else if (this.#legacyOperations !== null) {
const legacyOperation = Object.assign({}, original)
const legacyOperation = { ...original }

legacyOperation.type = 'del'
legacyOperation.key = key
Expand Down
42 changes: 21 additions & 21 deletions abstract-level.js
Original file line number Diff line number Diff line change
Expand Up @@ -58,7 +58,8 @@ class AbstractLevel extends EventEmitter {
permanence: manifest.permanence !== false,

encodings: manifest.encodings || {},
events: Object.assign({}, manifest.events, {
events: {
...manifest.events,
opening: true,
open: true,
closing: true,
Expand All @@ -68,7 +69,7 @@ class AbstractLevel extends EventEmitter {
del: true,
batch: true,
clear: true
})
}
})

// Monitor event listeners
Expand Down Expand Up @@ -332,8 +333,7 @@ class AbstractLevel extends EventEmitter {

// Forward encoding options to the underlying store
if (options.keyEncoding !== keyFormat || options.valueEncoding !== valueFormat) {
// Avoid spread operator because of https://bugs.chromium.org/p/chromium/issues/detail?id=1204540
options = Object.assign({}, options, { keyEncoding: keyFormat, valueEncoding: valueFormat })
options = { ...options, keyEncoding: keyFormat, valueEncoding: valueFormat }
}

const encodedKey = keyEncoding.encode(key)
Expand Down Expand Up @@ -390,7 +390,7 @@ class AbstractLevel extends EventEmitter {

// Forward encoding options
if (options.keyEncoding !== keyFormat || options.valueEncoding !== valueFormat) {
options = Object.assign({}, options, { keyEncoding: keyFormat, valueEncoding: valueFormat })
options = { ...options, keyEncoding: keyFormat, valueEncoding: valueFormat }
}

const mappedKeys = new Array(keys.length)
Expand Down Expand Up @@ -454,11 +454,10 @@ class AbstractLevel extends EventEmitter {

// Forward encoding options to the underlying store
if (options === this.#defaultOptions.key) {
// Avoid Object.assign() for default options
// Avoid cloning for default options
options = this.#defaultOptions.keyFormat
} else if (options.keyEncoding !== keyFormat) {
// Avoid spread operator because of https://bugs.chromium.org/p/chromium/issues/detail?id=1204540
options = Object.assign({}, options, { keyEncoding: keyFormat })
options = { ...options, keyEncoding: keyFormat }
}

const encodedKey = keyEncoding.encode(key)
Expand Down Expand Up @@ -504,11 +503,10 @@ class AbstractLevel extends EventEmitter {

// Forward encoding options to the underlying store
if (options === this.#defaultOptions.key) {
// Avoid Object.assign() for default options
// Avoid cloning for default options
options = this.#defaultOptions.keyFormat
} else if (options.keyEncoding !== keyFormat) {
// Avoid spread operator because of https://bugs.chromium.org/p/chromium/issues/detail?id=1204540
options = Object.assign({}, options, { keyEncoding: keyFormat })
options = { ...options, keyEncoding: keyFormat }
}

const mappedKeys = new Array(keys.length)
Expand Down Expand Up @@ -565,12 +563,12 @@ class AbstractLevel extends EventEmitter {
const enableWriteEvent = this.#eventMonitor.write
const original = options

// Avoid Object.assign() for default options
// Avoid cloning for default options
// TODO: also apply this tweak to get() and getMany()
if (options === this.#defaultOptions.entry) {
options = this.#defaultOptions.entryFormat
} else if (options.keyEncoding !== keyFormat || options.valueEncoding !== valueFormat) {
options = Object.assign({}, options, { keyEncoding: keyFormat, valueEncoding: valueFormat })
options = { ...options, keyEncoding: keyFormat, valueEncoding: valueFormat }
}

const encodedKey = keyEncoding.encode(key)
Expand All @@ -580,15 +578,16 @@ class AbstractLevel extends EventEmitter {
await this._put(prefixedKey, encodedValue, options)

if (enableWriteEvent) {
const op = Object.assign({}, original, {
const op = {
...original,
type: 'put',
key,
value,
keyEncoding,
valueEncoding,
encodedKey,
encodedValue
})
}

this.emit('write', [op])
} else {
Expand Down Expand Up @@ -622,11 +621,11 @@ class AbstractLevel extends EventEmitter {
const enableWriteEvent = this.#eventMonitor.write
const original = options

// Avoid Object.assign() for default options
// Avoid cloning for default options
if (options === this.#defaultOptions.key) {
options = this.#defaultOptions.keyFormat
} else if (options.keyEncoding !== keyFormat) {
options = Object.assign({}, options, { keyEncoding: keyFormat })
options = { ...options, keyEncoding: keyFormat }
}

const encodedKey = keyEncoding.encode(key)
Expand All @@ -635,12 +634,13 @@ class AbstractLevel extends EventEmitter {
await this._del(prefixedKey, options)

if (enableWriteEvent) {
const op = Object.assign({}, original, {
const op = {
...original,
type: 'del',
key,
keyEncoding,
encodedKey
})
}

this.emit('write', [op])
} else {
Expand Down Expand Up @@ -694,7 +694,7 @@ class AbstractLevel extends EventEmitter {
// Clone the op so that we can freely mutate it. We can't use a class because the
// op can have userland properties that we'd have to copy, negating the performance
// benefits of a class. So use a plain object.
const op = Object.assign({}, options, operations[i])
const op = { ...options, ...operations[i] }

// Hook functions can modify op but not its type or sublevel, so cache those
const isPut = op.type === 'put'
Expand Down Expand Up @@ -753,7 +753,7 @@ class AbstractLevel extends EventEmitter {
if (enableWriteEvent && !siblings) {
// Clone op before we mutate it for the private API
// TODO (future semver-major): consider sending this shape to private API too
publicOperation = Object.assign({}, op)
publicOperation = { ...op }
publicOperation.encodedKey = encodedKey

if (delegated) {
Expand Down
2 changes: 1 addition & 1 deletion lib/prewrite-batch.js
Original file line number Diff line number Diff line change
Expand Up @@ -58,7 +58,7 @@ class PrewriteBatch {
// If the sublevel is not a descendant then we shouldn't emit events
if (this.#publicOperations !== null && !siblings) {
// Clone op before we mutate it for the private API
publicOperation = Object.assign({}, op)
publicOperation = { ...op }
publicOperation.encodedKey = encodedKey

if (delegated) {
Expand Down
2 changes: 1 addition & 1 deletion test/iterator-range-test.js
Original file line number Diff line number Diff line change
Expand Up @@ -38,7 +38,7 @@ exports.range = function (test, testCommon) {
// Test the documented promise that in reverse mode,
// "the returned entries are the same, but in reverse".
if (!opts.reverse && !('limit' in opts)) {
const reverseOpts = Object.assign({}, opts, { reverse: true })
const reverseOpts = { ...opts, reverse: true }

rangeTest(
name + ' (flipped)',
Expand Down

0 comments on commit a5c2e52

Please sign in to comment.