Skip to content

Commit

Permalink
Fix non-first parameter merging when reconfiguring plugins
Browse files Browse the repository at this point in the history
If you configure a plugin with multiple parameters (which is supported
but not particularly recommended, or used), and then reconfigure it
multiple times, the non-first parameters (so everything other than
what’s typically the `options` object), we now take the last configured
values.
Previously, earlier values remained.
  • Loading branch information
wooorm committed Aug 14, 2023
1 parent 6f068a0 commit 56ee288
Show file tree
Hide file tree
Showing 2 changed files with 92 additions and 14 deletions.
27 changes: 13 additions & 14 deletions lib/index.js
Original file line number Diff line number Diff line change
Expand Up @@ -1210,29 +1210,28 @@ export class Processor extends CallableInstance {
*/
function addPlugin(plugin, parameters) {
let index = -1
/** @type {PluginTuple<Array<unknown>> | undefined} */
let entry
let entryIndex = -1

while (++index < attachers.length) {
if (attachers[index][0] === plugin) {
entry = attachers[index]
entryIndex = index
break
}
}

if (entry) {
let value = parameters[0]
if (isPlainObj(entry[1]) && isPlainObj(value)) {
value = structuredClone({...entry[1], ...value})
if (entryIndex === -1) {
attachers.push([plugin, ...parameters])
}
// Only set if there was at least a `primary` value, otherwise we’d change
// `arguments.length`.
else if (parameters.length > 0) {
let [primary, ...rest] = parameters
const currentPrimary = attachers[entryIndex][1]
if (isPlainObj(currentPrimary) && isPlainObj(primary)) {
primary = structuredClone({...currentPrimary, ...primary})
}

entry[1] = value
// To do: should the rest be set?
} else {
// It’s important to pass arguments, because an explicit passed
// `undefined` is different from a not-passed `value`.
// This way we keep things at their place.
attachers.push([plugin, ...parameters])
attachers[entryIndex] = [plugin, primary, ...rest]
}
}
}
Expand Down
79 changes: 79 additions & 0 deletions test/use.js
Original file line number Diff line number Diff line change
Expand Up @@ -1291,4 +1291,83 @@ test('`use`', async function (t) {
}
)
})

await t.test('reconfigure (non-first parameters)', async function (t) {
await t.test(
'should reconfigure plugins (non-first parameters)',
async function () {
let calls = 0

unified()
.use(fn, givenOptions, givenOptions, givenOptions, undefined)
.use(fn, otherOptions, otherOptions, undefined, otherOptions)
.freeze()

assert.equal(calls, 1)

/**
* @param {unknown} a
* @param {unknown} b
* @param {unknown} c
* @param {unknown} d
*/
function fn(a, b, c, d) {
assert.equal(arguments.length, 4)
assert.deepEqual(a, mergedOptions)
assert.deepEqual(b, otherOptions)
assert.deepEqual(c, undefined)
assert.deepEqual(d, otherOptions)
calls++
}
}
)

await t.test('should keep parameter length (#1)', async function () {
let calls = 0

unified().use(fn).use(fn).freeze()

assert.equal(calls, 1)

/**
* @param {...unknown} parameters
*/
function fn(...parameters) {
assert.deepEqual(parameters, [])
calls++
}
})

await t.test('should keep parameter length (#2)', async function () {
let calls = 0

unified().use(fn, givenOptions).use(fn).freeze()

assert.equal(calls, 1)

/**
* @param {...unknown} parameters
*/
function fn(...parameters) {
assert.deepEqual(parameters, [givenOptions])
calls++
}
})

await t.test('should keep parameter length (#3)', async function () {
let calls = 0

unified().use(fn).use(fn, givenOptions).freeze()

assert.equal(calls, 1)

/**
* @param {...unknown} parameters
*/
function fn(...parameters) {
assert.deepEqual(parameters, [givenOptions])
calls++
}
})
})
})

0 comments on commit 56ee288

Please sign in to comment.