Remove the dead config schema leg from the configuration merge - #1821
Merged
Merged
Conversation
Add regression tests for upgrading users before simplifying the legacy configuration merge. A realistic exported connection file fixture verifies import splits the bundled schema into schemaAtom and never writes it onto the config entry, and a backup/restore round-trip verifies a stored config carrying a stray legacy schema field survives losslessly.
mergeConfiguration merged schema from three sources: the connection config's own schema, the synced schema, and user styling. No released writer ever populates a connection config's schema (import splits it into schemaAtom, schema sync writes there, and default/create connections never set it), so that merge leg was always empty. Drop it, leaving a two-source merge of synced schema over defaults with user styling applied. Each type now has a single schema source, so the union-of-keys logic, the mergeAttributes helper, and the unknown-type fallbacks collapse away. Behavior is unchanged; the merge tests pass untouched. activeConfigSelector is no longer exported since nothing outside this module used it.
5 tasks
kmcginnes
marked this pull request as ready for review
June 15, 2026 15:48
arseny-kostenko
approved these changes
Jun 17, 2026
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.
Description
mergeConfigurationpreviously merged schema type configs from three sources: the connection config's ownschema, the synced schema, and user styling. The first source was always empty — no released code path ever populatesRawConfiguration.schema(import splits the bundled schema intoschemaAtom, schema sync writes there, and create/default connections never set it). The union-of-keys logic, themergeAttributeshelper, and the unknown-type fallbacks all existed only to service that dead leg.This drops the dead source, leaving a straightforward two-source merge: synced schema over defaults, with user styling applied on top. Each type now has a single schema source, so those helpers collapse away (−91/+14 in
configuration.ts). Behavior is unchanged.How to read:
configuration.ts) — the actual change.Core vs supporting:
configuration.tssimplification.schemaAtomand never writes it onto the config entry; a backup/restore round-trip preserves a stored config carrying a stray legacy schema field), a faithfully-shaped test fixture, and a cleanup of the surrounding import tests to share one lookup helper.Validation
pnpm checkspasses (lint, format, types).pnpm testpasses (1789 tests). The pre-existing merge tests pass untouched, which is the primary evidence the refactor is behavior-preserving.BACKWARD COMPATIBILITY — PERSISTED DATAtests were authored before the refactor and verified to fail loudly if the invariants break (mutation-tested).Related Issues
Check List
pnpm checkspasses with no errors.pnpm testpasses with no failures.