Integrate community PRs #10, #11, and #12 - #17
Merged
Conversation
Extracts workSourceSchema, workStateCategorySchema, and trackerViewSchema into a leaf work-schemas.ts module with no local imports of its own (re-exported from contract.ts and board-settings.ts unchanged for existing importers) and adds filter-state.ts, defining the persisted board filter shape, per-project scope-key collapsing, and normalization/fingerprinting so unordered array changes don't look like state changes. work-schemas.ts exists because filter-state.ts is the first module in this plugin that is both directly unit-tested and dependent on real (non-type-only) values from a sibling module. Every other tested module only crosses files via `import type`, which is erased before node --experimental-strip-types ever tries to resolve it; that loader never rewrites a .js specifier to a sibling .ts file, so a real value import chain through board-settings.ts (which itself imports credential-contract.js and browse.js) would not resolve under the raw test runner. Concentrating the primitive enums in a dependency-free leaf module lets filter-state.ts import real values by depending on that leaf directly (via an explicit .ts specifier, since it's a new file loaded raw by tests) without needing any other existing file to change its import style. board-settings.ts keeps its original .js specifiers throughout and only gains a re-export of trackerViewSchema/TrackerView from the new leaf. Review fixes folded in: - Added filter-state.ts and work-schemas.ts to package.json files, in alphabetical position; npm pack was previously missing work-schemas.ts even though contract.ts and board-settings.ts both depend on it. - Extended the normalization test to assert every one of the seven array fields sorts and dedupes independently, with a distinct value per field, so a copy/paste cross-wiring in normalizeBoardFilterState fails loudly. - filterStateScopeId no longer aliases an empty scope onto the across-projects row; it now falls through and is rejected downstream by bbProjectIdSchema like any other malformed scope, with a test and a comment noting the (unguarded) collision risk if a real bb project were ever literally named proj_across_projects. - Removed the unused `type WorkStateCategory` from contract.ts's value import block (only the separate `export type` line needs it). - Pinned the strictness test to safeParse + the `unrecognized_keys` issue code instead of an unpredicated assert.throws.
Returning to the board from a thread cleared that project's filters while leaving other projects intact. The saved row was never corrupted: the state was fetched correctly and then thrown away. The parent re-reads initialPreferences from a mutable Map on every render and passes it as a prop, and it sat in the load effect's dependency array. The effect that records preferences ran on mount, before the load resolved, so it cached an empty placeholder. That flipped initialPreferences from undefined to a truthy empty object, re-ran the load, cancelled the in-flight fetch, and the second pass then hit `if (initialPreferences) return` and refused to apply the saved state. Two fetches, zero applications. Switching projects was unaffected because it changes projectId, landing on a scope whose cache entry was absent or already real. Only the thread round trip re-rendered the parent without changing project. - Capture initialPreferences in a ref at mount. It seeds useState; it has no business re-running the load. - Refuse to cache a preferences snapshot until the load has resolved, so an empty placeholder can never masquerade as in-session state. Confirmed against the running plugin: one getBoardFilterState per mount instead of two, and the filters survive the round trip. Co-Authored-By: Claude Opus 5 (1M context) <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
Verification
npm run checkmainProvenance
The exact heads of PRs #10, #11, and #12 are ancestors of this branch. Please merge this PR normally (not squash/rebase) so the original contributor commits and authors remain reachable.
No package version, tag, npm, release, or marketplace change is included.