Add isLoading checking for the Example field in Create Listing Modal by lucaslyl · Pull Request #4250 · cardstack/boxel

lucaslyl · 2026-03-26T06:13:57Z

linear: https://linear.app/cardstack/issue/CS-10532/fix-example-missing-in-the-create-listing-modal

Add targetRealm and isLive options to the create listing modal's search resource so instances are scoped to the correct realm with live updates
Hide examples picker while search is loading to prevent showing stale/empty state
Fix search resource to properly clean up subscriptions and cached state when query becomes undefined (e.g. modal close), preventing subscription leaks
Add integration tests verifying that setting query to undefined clears cached state and tears down live subscriptions

lucaslyl · 2026-03-26T06:16:00Z

packages/host/app/components/operator-mode/create-listing-modal.gts

-          {{/if}}
+          {{#unless this.instancesSearch.isLoading}}
+            {{#if this.instanceOptions.length}}
+              <FieldContainer @label='Examples' class='field'>


add loading checking and also the instance length check else the fieldcontainer will still display if the search result is empty

chatgpt-codex-connector

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: dbaa69cd63

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

Open a pull request for review
Mark a draft as ready
Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

chatgpt-codex-connector · 2026-03-26T06:17:39Z

packages/host/app/components/operator-mode/create-listing-modal.gts

+    getOwner(this)!,
+    () => (this.codeRef ? { filter: { type: this.codeRef } } : undefined),
+    undefined,
+    { isLive: true },


Avoid enabling live search without unsubscribe on close

Setting isLive: true here introduces a persistent background subscription after the modal is dismissed. CreateListingModal is always mounted (operator-mode/container.gts renders it unconditionally), and when the payload is cleared codeRef becomes undefined; in SearchResource.modify() (packages/host/app/resources/search.ts) that path returns early on query === undefined without unsubscribing existing live subscriptions or clearing #previousQuery. As a result, later realm index events can continue triggering search.perform(...) for the stale query even while the modal is closed, causing unnecessary live refresh traffic/work until the whole component is destroyed.

Useful? React with 👍 / 👎.

github-actions · 2026-03-26T06:20:45Z

Preview deployments

github-actions · 2026-03-26T06:36:16Z

Host Test Results

1 files ±0 1 suites ±0 2h 7m 1s ⏱️ - 3m 35s
2 058 tests +2 2 043 ✅ +2 15 💤 ±0 0 ❌ ±0
2 073 runs +2 2 058 ✅ +2 15 💤 ±0 0 ❌ ±0

Results for commit 8f19f26. ± Comparison against base commit a2591cf.

♻️ This comment has been updated with latest results.

Copilot

Pull request overview

This PR improves the “Create Listing” modal’s Examples behavior by switching its instance lookup to live search, and tightens the search resource’s live-subscription lifecycle so subscriptions are torn down when the query is cleared.

Changes:

Add live-subscription teardown when SearchResource’s query becomes undefined, plus a helper for unsubscribing.
Add an integration test to verify live search stops reacting to realm index events after the query is cleared.
Update Create Listing modal to use live search for instances and hide the Examples field while the search is loading.

Reviewed changes

Copilot reviewed 3 out of 3 changed files in this pull request and generated 3 comments.

File	Description
`packages/host/app/resources/search.ts`	Adds `teardownLiveSubscriptions()` and clears live subscriptions when the query is absent.
`packages/host/tests/integration/resources/search-test.ts`	Adds coverage ensuring live searches stop re-fetching after clearing the query.
`packages/host/app/components/operator-mode/create-listing-modal.gts`	Enables live search for example instances and guards the Examples field behind `isLoading`.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Copilot · 2026-03-26T22:15:34Z

packages/host/app/resources/search.ts

+  private teardownLiveSubscriptions() {
+    for (let subscription of this.subscriptions) {
+      subscription.unsubscribe();
+    }
+    this.subscriptions = [];
+  }


SearchResource.modify() never tears down existing realm subscriptions when isLive flips from true to false (while query remains defined). In that case this.subscriptions stays active and the callback will keep triggering search.perform(...) even though the resource is no longer live. Consider calling teardownLiveSubscriptions() when !isLive (or when transitioning from live→non-live) to ensure subscriptions reflect the current args.

Copilot · 2026-03-26T22:15:34Z

packages/host/app/components/operator-mode/create-listing-modal.gts

+    getOwner(this)!,
+    () => (this.codeRef ? { filter: { type: this.codeRef } } : undefined),
+    undefined,
+    { isLive: true },


This getSearch(..., { isLive: true }) call does not provide realms, so the search resource will treat it as “all available realms” and will subscribe live to every realm in realmServer.availableRealmURLs. That can be very expensive (many live subscriptions + refreshes) for a modal; consider constraining realms (e.g. targetRealm and/or the realm of codeRef.module) or keeping this search non-live unless you explicitly need cross-realm live updates.

Suggested change

{ isLive: true },

{ isLive: false },

Copilot · 2026-03-26T22:15:34Z

packages/host/app/components/operator-mode/create-listing-modal.gts

+          {{#unless this.instancesSearch.isLoading}}
+            {{#if this.instanceOptions.length}}
+              <FieldContainer @label='Examples' class='field'>
+                <div class='field-contents' data-test-examples-container>
+                  <CardInstancePicker
+                    @placeholder='Select examples to include in the listing'
+                    @options={{this.instanceOptions}}
+                    @selected={{this.effectiveSelected}}
+                    @onChange={{this.onExampleChange}}
+                    @maxSelectedDisplay={{3}}
+                    data-test-create-listing-examples
+                  />
+                </div>
+              </FieldContainer>
+            {{/if}}
+          {{/unless}}


The Examples picker is now hidden while instancesSearch.isLoading, but the modal’s Create button remains enabled. If the user clicks Create before the search finishes, selectedExampleURLs can resolve to an empty list (because instances/instanceOptions haven’t populated yet), resulting in a listing created without the user’s intended examples. Consider disabling Create while instancesSearch.isLoading and/or having selectedExampleURLs fall back to payload.openCardIds when options haven’t loaded yet.

make sense ,agree with this

…scription leak

lucaslyl · 2026-03-27T02:51:02Z

@codex review

chatgpt-codex-connector · 2026-03-27T02:54:59Z

Codex Review: Didn't find any major issues. Keep them coming!

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

Open a pull request for review
Mark a draft as ready
Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

Copilot

Pull request overview

Copilot reviewed 3 out of 3 changed files in this pull request and generated 4 comments.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Copilot · 2026-03-27T07:48:04Z

packages/host/tests/integration/resources/search-test.ts

+    // Simulate modal close: set query to undefined
+    args = { ...args, query: undefined as any };
+    search.modify([], args);


Using query: undefined as any weakens type-safety in a place where query is already typed as Query | undefined. Prefer passing plain undefined (and if the satisfies SearchResourceArgs['named'] constraint blocks that, it’s a sign the args type should be adjusted rather than casting away types).

Copilot · 2026-03-27T07:48:04Z

packages/host/tests/integration/resources/search-test.ts

+    );
+
+    // Simulate modal close: set query to undefined
+    args = { ...args, query: undefined as any };


Same here: avoid query: undefined as any—it bypasses the intended Query | undefined typing and can mask real type regressions in the test.

Suggested change

args = { ...args, query: undefined as any };

args = { ...args, query: undefined };

Copilot · 2026-03-27T07:48:04Z

packages/host/app/resources/search.ts

+      for (let subscription of this.subscriptions) {
+        subscription.unsubscribe();
+      }
+      this.subscriptions = [];


When query becomes undefined you unsubscribe and clear the previous query signature, but the resource still retains the prior _instances (and their store references) plus _meta/_errors, and any in-flight search task will keep running and may repopulate state after “close”. If this is meant to model modal close, consider also canceling any running tasks and clearing instances/metadata while dropping references (e.g., drop references for all current instance ids, empty _instances, reset _meta/_errors, and optionally clear realmsToSearch) to avoid stale results and leaking store references while the resource remains alive.

Suggested change

this.subscriptions = [];

this.subscriptions = [];

// cancel any in-flight search task so it cannot repopulate state after close

let searchTaskInstance = (this as any).search;

if (searchTaskInstance && typeof searchTaskInstance.cancelAll === 'function') {

searchTaskInstance.cancelAll();

}

// clear prior instances, metadata, and errors to avoid stale state and leaked references

(this as any)._instances = {};

(this as any)._meta = undefined;

(this as any)._errors = undefined;

this.realmsToSearch = [];

Copilot · 2026-03-27T07:48:04Z

packages/host/tests/integration/resources/search-test.ts

+  test(`setting query to undefined clears cached state and re-runs search on next query`, async function (assert) {
+    let query: Query | undefined = {
+      filter: {
+        on: {
+          module: `${testRealmURL}book`,
+          name: 'Book',
+        },
+        eq: {
+          'author.lastName': 'Abdel-Rahman',
+        },
+      },
+    };
+    let args = {
+      query,
+      realms: [testRealmURL],
+      isLive: false,
+      isAutoSaved: false,
+      storeService,
+      owner: this.owner,
+    } satisfies SearchResourceArgs['named'];
+
+    let search = getSearchResourceForTest(loaderService, () => ({
+      named: args,
+    }));
+    await search.loaded;
+    assert.strictEqual(
+      search.instances.length,
+      2,
+      'initial search returns 2 books',
+    );
+
+    // Simulate modal close: set query to undefined
+    args = { ...args, query: undefined as any };
+    search.modify([], args);
+    await settled();
+
+    // Simulate modal reopen with same query
+    args = { ...args, query };
+    search.modify([], args);
+    await search.loaded;
+
+    assert.strictEqual(
+      search.instances.length,
+      2,
+      'search re-runs after query was set to undefined and back',
+    );
+  });


This test doesn’t actually prove that the search re-runs after toggling query to undefined: with the previous behavior (no cache reset), instances would still be length 2 and await search.loaded would still resolve (because it’s the already-resolved promise), so the assertions would still pass. To make this test fail under the old bug, assert that cached state is cleared on query: undefined (e.g., instances.length becomes 0 and/or meta/errors reset), or instrument the store/resource to assert a new search was performed (e.g., count calls or verify loaded changes to a new promise).

lucaslyl self-assigned this Mar 26, 2026

lucaslyl commented Mar 26, 2026

View reviewed changes

lucaslyl requested a review from a team March 26, 2026 06:16

chatgpt-codex-connector bot reviewed Mar 26, 2026

View reviewed changes

habdelra requested a review from Copilot March 26, 2026 22:07

Copilot started reviewing on behalf of habdelra March 26, 2026 22:10 View session

Copilot AI reviewed Mar 26, 2026

View reviewed changes

Fix missing examples in create listing modal and search resource sub…

85c5592

…scription leak

lucaslyl force-pushed the CS-10532/fix-example-missing-in-the-create-listing-modal branch from 0fc3e47 to 85c5592 Compare March 27, 2026 02:49

fix lint

8f19f26

richardhjtan requested a review from Copilot March 27, 2026 07:41

Copilot started reviewing on behalf of richardhjtan March 27, 2026 07:44 View session

Copilot AI reviewed Mar 27, 2026

View reviewed changes

	args = { ...args, query: undefined as any };
	args = { ...args, query: undefined };

-      this.subscriptions = [];
+      this.subscriptions = [];
+      // cancel any in-flight search task so it cannot repopulate state after close
+      let searchTaskInstance = (this as any).search;
+      if (searchTaskInstance && typeof searchTaskInstance.cancelAll === 'function') {
+        searchTaskInstance.cancelAll();
+      }
+      // clear prior instances, metadata, and errors to avoid stale state and leaked references
+      (this as any)._instances = {};
+      (this as any)._meta = undefined;
+      (this as any)._errors = undefined;
+      this.realmsToSearch = [];

Conversation

lucaslyl commented Mar 26, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

lucaslyl Mar 26, 2026

Choose a reason for hiding this comment

Uh oh!

chatgpt-codex-connector bot left a comment

Choose a reason for hiding this comment

💡 Codex Review

Uh oh!

chatgpt-codex-connector bot Mar 26, 2026

Choose a reason for hiding this comment

Uh oh!

github-actions bot commented Mar 26, 2026

Preview deployments

Uh oh!

github-actions bot commented Mar 26, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Host Test Results

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Reviewed changes

Uh oh!

Copilot AI Mar 26, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Mar 26, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Mar 26, 2026

Choose a reason for hiding this comment

Uh oh!

lucaslyl Mar 27, 2026

Choose a reason for hiding this comment

Uh oh!

lucaslyl commented Mar 27, 2026

Uh oh!

chatgpt-codex-connector bot commented Mar 27, 2026

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Uh oh!

Copilot AI Mar 27, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Mar 27, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Mar 27, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Mar 27, 2026

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

lucaslyl commented Mar 26, 2026 •

edited

Loading

github-actions bot commented Mar 26, 2026 •

edited

Loading