fix(applications): preserve in-table comments in the visual schema editor#1468
Open
dawsontoth wants to merge 1 commit into
Open
fix(applications): preserve in-table comments in the visual schema editor#1468dawsontoth wants to merge 1 commit into
dawsontoth wants to merge 1 commit into
Conversation
…itor The visual schema editor keeps untouched tables byte-for-byte but regenerates a table from its parsed model the moment it's edited. The model had nowhere to store comments that live inside a `@table` block but aren't attached above a field, so editing a table silently dropped or relocated them. Because unedited files round-trip perfectly, the loss only surfaced on the first edit — an inconsistent state (studio #1462). Three failure modes, all now covered by regression tests: - A comment trailing the last field (before `}`) was accumulated as pending trivia and discarded when no field followed. `parseFields` now returns the leftover as `trailingComments`; `generateTable` re-emits it before `}`. - A comments-only body dropped the comment AND emitted `type X @table {\n}` — an empty field block, which is a hard graphql-js syntax error that makes Harper reject the whole schema (the "table never loads" symptom). To keep that state unreachable, the visual editor now blocks removing a table's last field and flags a zero-field table with a warning. - A header-line comment (`type X @table { # note`) was relocated to a standalone line above the first field. It's now captured as `headerComment` and re-emitted on the header line. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Code Review
This pull request introduces UI constraints to prevent the removal of the last field in a table, ensuring GraphQL types remain valid with at least one field. Additionally, it enhances the schema parser and serializer to preserve header-line and trailing comments inside table bodies during edits, backed by comprehensive unit tests. There are no review comments to address, so I have no feedback to provide.
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
1 participant
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.
Fixes #1462.
Problem
The visual schema editor keeps untouched tables byte-for-byte, but regenerates a table from its parsed model the moment it's edited. The model had nowhere to store comments that live inside a
@tableblock but aren't attached above a field, so editing a table silently dropped or relocated them. Because unedited files round-trip perfectly, the loss only surfaced on the first edit — the "inconsistent state" from the issue.Three failure modes, reproduced then fixed (each now a regression test):
}type X @table {\n}— an empty field block, which is a hard graphql-js syntax error that makes Harper reject the whole schema (the "table never loads to the database" symptom)type X @table { # noteFix
types.ts): addedtrailingComments: string[]andheaderComment?: stringtoTableModel.parseSchema.ts):parseFieldsnow returns leftover trivia astrailingCommentsinstead of discarding it; newsplitHeaderCommentpeels a header-line comment off the body so it isn't misattributed to the first field.serializeSchema.ts):generateTablere-emits the header comment on the header line and trailing comments before}(sharedindentCommentLineshelper).TableCard.tsx,FieldRow.tsx): the remove button on a table's last field is disabled (a GraphQL type needs ≥1 field), and a zero-field table shows a warning in both the summary line and the field list — so the invalid, schema-breaking state is unreachable through normal editing and surfaced when a file already contains it.Design note
For the empty-body case I chose a UI guard + visible warning rather than blocking Save or silently dropping a zero-field table — matching this codebase's existing pattern (invalid type names are flagged with a hint, not hard-blocked). Silently dropping a table the user is actively editing would be its own inconsistent-state bug. Happy to add a hard Save-time block if preferred.
Testing
parseSchema.test.ts/serializeSchema.test.tsencode the exact three reproductions (edit → serialize) — they fail on the old code.tsc, oxlint, and commitlint clean.🤖 Generated with Claude Code