Repository navigation
Conversation
… 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
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
What happens
The lower bound is not checked at all when
innercarries adefault:maxis unaffected, so onlyminis 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: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.vueandutils.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 andskipMinkept 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 thepop()on its own —
+0/-1, one line, unrelated commit message — and leftskipMinbehind:- 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
minbefore the value is validated, so it does not depend on the guard.Test
Added to the existing
array (length)test inpackages/core/tests/array.spec.tsrather than a new file:
yarn yakumo test core, with the test present and the source change reverted:and with the change applied:
The one remaining failure is
I18n > basic support(i18n.spec.ts:46), which fails onunmodified
mainas well: