Repository navigation
Conversation
🦋 Changeset detectedLatest commit: 088af11 The changes in this PR will be included in the next version bump. This PR includes changesets to release 1 package
Not sure what this means? Click here to learn what changesets are. Click here if you're a maintainer who wants to add another changeset to this PR |
There was a problem hiding this comment.
🟢 Approval recommended
No active review issues were identified for the legacy configuration-normalization changes.
No open issues 🎉
What changed in this PR
Hardens legacy configuration normalization for direct JavaScript callers of the public PluginConfigurationManager, without changing the editor configuration protocol or listener lifecycle.
Changes:
- Copies accepted tag arrays and top-level configuration records while preserving nested references and unknown lint keys.
- Validates array lengths and recovers from configuration read failures by restoring defaults, logging safely, and notifying listeners.
- Adds regression coverage, documents the behavior, and includes a patch changeset.
| File | Description |
|---|---|
test/unit/plugin-configuration.test.ts |
Adds regression cases for copying, special lint keys, invalid lengths, and throwing inputs. |
src/configuration/plugin-configuration.ts |
Adds guarded normalization, shallow copies, safe lint property writes, and array-length validation. |
docs/architecture.md |
Documents recovery, copying, and listener behavior. |
.changeset/steady-legacy-configuration.md |
Records the configuration fixes for a patch release. |
Review effort: Balanced
There was a problem hiding this comment.
✅ No new issues found.
Reviewed changes
- Reset on unreadable input:
updateFromPluginConfignow wraps normalization intry/catch; a throwing getter, revoked proxy, or throwinglengthresets the whole configuration to defaults, logs one message, and still notifies listeners exactly once with listener callbacks kept outside the guard. - Mutation independence: accepted tag arrays are copied, and
emmet/mergedlintrecords are shallow-copied viaObject.fromEntries, while nested values stay shared andlint.validPropertiesis rebuilt as a new array. - Safe array length: new
getArrayLengthrejects arrays whose reported length is not a nonnegative safe integer before iteration, applied to bothtagsandlint.validProperties. - Own-key preservation: unknown lint keys are written with
Object.defineProperty, so__proto__survives as an own data property without changing the record's prototype. - Tests/docs/changeset: seven targeted regression cases, an updated Configuration section in
docs/architecture.md, and a patch changeset.
I re-ran the full unit suite (942 pass) and yarn typecheck, and confirmed the seven new tests fail when only src/configuration/plugin-configuration.ts is reverted to the base commit. The __proto__ behavior is preserved end to end (spread and Object.fromEntries both create an own data property) and is inert in the downstream vscode-css-languageservice lint configuration, which only reads own keys.
big-pickle (free) | 𝕏

Summary
Direct callers of the public
PluginConfigurationManagercan pass JavaScript objects rather than JSON. A throwing getter or revoked proxy currently escapes normalization, caller mutations can change accepted tag arrays or Emmet options without a configuration update, and an unknown__proto__lint key is lost.Copy accepted tag arrays and top-level configuration records, retain nested object references, and preserve unknown lint keys as own data properties. Validate array lengths before normalizing entries; logging a rejected value can still read its properties. If reading or normalizing configuration throws, reset to defaults, report the problem through the existing safe logger, and notify configuration listeners once. Listener callbacks remain outside the normalization catch.
This change contains the legacy normalization work separated from PR #43: the normalization guard,
getArrayLength, own-property lint writes, andcloneConfigurationRecord. It is based directly on main and introduces no editor configuration protocol types, listener lifecycle changes, or diagnostic refresh changes.Testing
corepack yarn test:unit -t 'PluginConfigurationManager'corepack yarn verifyThe seven new regression cases failed on main before the implementation change and pass afterward. The full
verifyrun passes.