fix(static-config): treat absent keys as NonStatic when object has spreads#788
Merged
branchseer merged 7 commits intomainfrom Mar 12, 2026
Merged
fix(static-config): treat absent keys as NonStatic when object has spreads#788branchseer merged 7 commits intomainfrom
branchseer merged 7 commits intomainfrom
Conversation
fengmk2
approved these changes
Mar 12, 2026
Member
|
@branchseer let's fix tests and merge, I want to release a new version. |
✅ Deploy Preview for viteplus-staging canceled.
|
Member
Author
@fengmk2 Fix pushed. Let's see how it goes. |
…reads
Previously, keys absent from the extracted field map were always treated as
"field definitely doesn't exist" — but this was wrong when the object contained
spread or computed-key properties, since the spread may introduce any key.
Introduce `FieldMap` (opaque struct) backed by a `FieldMapInner` enum:
- `Closed(FxHashMap<Box<str>, FieldValue>)` — no spreads or computed keys;
the map is exhaustive and absent keys are definitively absent.
- `Open(FxHashMap<Box<str>, serde_json::Value>)` — at least one spread or
computed-key property was present; only Json values for keys explicitly
declared *after* the last such entry are stored; `get()` returns
`Some(NonStatic)` for any other key.
When a spread or computed key is encountered the pre-spread accumulator is
cleared (entries would all be NonStatic anyway, implied by the Open variant).
Only Json values are stored in the Open map — NonStatic is redundant there.
Fixes: `{ ...base }` where `run` is absent from the map but may exist in
`base` — the consumer now correctly falls back to NAPI instead of silently
treating `run` as non-existent.
Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
is_empty was only used in two tests and had no production callers.
The same assertions are expressed with get("run").is_none().
Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Previously, `extract_object_fields` left keys absent from the result map when a spread (`...base`) or computed key was present. The consumer treated absent as "field definitively doesn't exist", silently skipping NAPI even though the spread may have introduced the field. Fix by introducing an opaque `FieldMap` type backed by two internal variants: - `Closed` — no spreads/computed keys; absent → `None` (field doesn't exist) - `Open` — had at least one spread/computed key; absent → `Some(NonStatic)` (field may exist via the spread, NAPI fallback triggered) `StaticConfig` is now just `FieldMap` (no `Option` wrapper); the "not analyzable" case is represented by `FieldMap::unanalyzable()` (Open empty map), which causes every key lookup to return `Some(NonStatic)` and trigger NAPI. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Replace the dual-map approach (closed + open + has_unknown_keys flag) with a single `FieldMapInner` that transitions from `Closed` to `Open` on the first spread or computed key. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
- Replace `pub type StaticConfig = FieldMap` with `FieldMap` directly — the alias added no semantic value and required extra documentation to compensate for what the type itself already expresses. - Pre-size the HashMap in `parse_json_config` with the known object length to avoid growth reallocations. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
branchseer
force-pushed
the
fix/static-config-spread-last
branch
from
aaa92e7 to
242af9c
Compare
Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
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
2 participants
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.
Summary
Fixes a correctness bug in the static vite config extractor: when the config object contained a spread (
...base) or computed key ([k]: v), any field not explicitly declared after it was left absent from the result map — and the consumer treated absent as "field definitely doesn't exist", skipping NAPI entirely even when the spread might have introduced that field.Root cause
Fix
Introduce
FieldMap(opaque struct) backed by aFieldMapInnerenum with two variants:Closed— no spreads or computed keys; map is exhaustive;get()returnsNonefor absent keys (field definitely absent).Open— had at least one spread or computed key;get()returnsSome(NonStatic)for absent keys (may exist via the spread); onlyJsonvalues for keys explicitly declared after the last spread are stored.When a spread is encountered, the pre-spread accumulator is cleared (not marked
NonStaticone by one — those entries would all be redundant in anOpenmap since absent already returnsNonStatic).Fields explicitly declared after the last spread are still extractable as
Json.Test coverage
spread_only:{ ...base }→get("run") = Some(NonStatic)(was wronglyNone)spread_then_explicit_run:{ ...base, run: { cacheScripts: true } }→get("run") = Some(Json({...}))(post-spread still extracted)no_spread_absent_is_none:{ plugins: [] }→get("run") = None(Closed, definitively absent)53 unit tests, all passing.
🤖 Generated with Claude Code