Skip to content

fix(core): enforce the array length limit when the inner schema has a default - #81

Open
173787247 wants to merge 1 commit into
shigma:mainfrom
173787247:core-array-length-defaulted-inner
Open

173787247 wants to merge 1 commit into
shigma:mainfrom
173787247:core-array-length-defaulted-inner

Conversation

@173787247

Copy link
Copy Markdown

What happens

The lower bound is not checked at all when inner carries a default:

Schema.array(Schema.string()).min(2)([])                 // throws: expected array length >= 2 but got 0
Schema.array(Schema.string().default('x')).min(2)([])    // returns []   ← the bound is skipped

max is unaffected, so only min is dropped. It is also not limited to scalar inners —
Schema.array(Schema.object({ a: Schema.string().default('y') })).min(2)([]) passes too.

packages/core/src/index.ts:654:

checkWithinRange(data.length, meta, 'array length', options, !isNullable(inner!.meta.default))
//                                                          ^ skipMin

Where the guard came from

0de846e3 (feat: enhance length check for array) added two things in the same hunk,
and also touched form/src/extensions/group.vue, table.vue and utils.ts:

+ while (data.length && isNullable(data[data.length - 1])) data.pop()
  checkWithinRange(data.length, meta, 'array length', !isNullable(inner!.meta.default))

The form pads short arrays with blank rows (packages/form/src/utils.ts:119-122, result.push(['' + result.length, null])),
so the pop() dropped those rows and skipMin kept the now-shorter array from failing the bound.
The pair was self-consistent.

f61fafda (fix: fix indexing for group entries, fix koishijs/koishi#1381) removed the pop()
on its own — +0/-1, one line, unrelated commit message — and left skipMin behind:

- while (data.length && isNullable(data[data.length - 1])) data.pop()
  checkWithinRange(data.length, meta, 'array length', !isNullable(inner!.meta.default))

Removing it looks right: it mutated the caller's array. But the guard it was paired with
has been skipping the bound for every defaulted inner schema since then, not only for padded input.

The form still pads to min before the value is validated, so it does not depend on the guard.

Test

Added to the existing array (length) test in packages/core/tests/array.spec.ts
rather than a new file:

test('array (length) with a defaulted inner', () => {
  const Config = Schema.array(Schema.string().default('x')).min(2)

  expect(() => new Config([])).to.throw()
  expect(() => new Config(['dress'])).to.throw()

  expect(new Config(['dress', 'skirt'])).to.deep.equal(['dress', 'skirt'])
  expect(new Config(['dress', null])).to.deep.equal(['dress', 'x'])
})

yarn yakumo test core, with the test present and the source change reverted:

ℹ tests 48    ℹ pass 45    ℹ fail 2
✖ array (length) with a defaulted inner
  Error [AssertionError]: expected [Function] to throw an error

and with the change applied:

ℹ tests 48    ℹ pass 46    ℹ fail 1
✔ array (length) with a defaulted inner (1.01ms)

The one remaining failure is I18n > basic support (i18n.spec.ts:46), which fails on
unmodified main as well:

# baseline, no changes
ℹ tests 47    ℹ pass 45    ℹ fail 1    ℹ skipped 1

… default

`Schema.array(inner).min(n)` skipped the lower bound whenever `inner` carried a
default, because the check was given `skipMin = !isNullable(inner.meta.default)`.

`skipMin` and a `while (data.length && isNullable(...)) data.pop()` were added
together for the form UI, which pads short arrays with blank rows: the `pop()`
removed those rows, and `skipMin` kept the now-shorter array from failing the
bound. The `pop()` was removed on its own six days later (it mutated the caller's
array), and the guard stayed behind, so the bound is now skipped for every
defaulted inner schema rather than only for padded input.

The form still pads to `min` before validating, so its arrays do not rely on the
guard. The added test fails on the current source and passes with this change.

This branch has not been deployed

No deployments
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Bug: Schema.array() 的添加项目按钮行为不正常,在特定场景不可用等

1 participant