Add FileDef GC coverage #3894
Open
Add FileDef GC coverage #3894
+105
−43
Conversation
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
lukemelia
force-pushed
the
file-meta-gc
branch
from
2d5f130 to
f42e772
Compare
Preview deployments |
Contributor
There was a problem hiding this comment.
Pull request overview
This PR extends the garbage collection (GC) mechanism in CardStoreWithGarbageCollection to include FileDef instances, which previously only participated in storage but not in GC and reference counting.
Changes:
- Renamed internal variables from "card" terminology to "instance" terminology to reflect that the maps now store both CardDef and FileDef instances
- Updated the
sweepmethod to handle FileDef instances separately from CardDef instances, since FileDef doesn't participate in the dependency graph - Enhanced the
hasReferencesmethod to work with both localIds (for CardDef) and remote URLs (for FileDef)
Reviewed changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated 1 comment.
| File | Description |
|---|---|
| packages/host/tests/integration/store-test.gts | Added two test cases verifying that FileDef instances are retained when referenced and garbage collected when references drop |
| packages/host/app/lib/gc-card-store.ts | Extended GC logic to handle FileDef instances, renamed variables for clarity, and updated hasReferences to work with both card and file identifiers |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
lukemelia
force-pushed
the
file-meta-gc
branch
from
f42e772 to
cc712af
Compare
habdelra
reviewed
Jan 23, 2026
| #nonTrackedInstances = new Map<string, StoredInstance>(); | ||
| #nonTrackedInstanceErrors = new Map<string, CardErrorJSONAPI>(); | ||
|
|
||
| #cards = new TrackedMap<string, StoredInstance>(); |
Contributor
There was a problem hiding this comment.
to be consistent we should probably rename this to instances
Contributor
There was a problem hiding this comment.
i noticed u didn't change this--was that an oversight?
habdelra
reviewed
Jan 23, 2026
lukemelia
force-pushed
the
file-meta-gc
branch
from
cc712af to
74c2cf2
Compare
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
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.
CardStoreWithGarbageCollection, but GC and reference counting only consider cards.This PR creates the following behavior: