fix(use-external-store-runtime): use-effects dependency

[hitmands]

This commit ensures that runtime.setAdapter is called again if the reference to store changes.

---

Important

Fixes useEffect dependency in useExternalStoreRuntime to update runtime.setAdapter when store changes.

• Behavior:
  • Fixes useEffect dependency in useExternalStoreRuntime to include store, ensuring runtime.setAdapter is called when store changes.

^{This description was created by for ecd1b10. You can customize this summary. It will automatically update as commits are pushed.}

[changeset-bot]

⚠️ No Changeset found

Latest commit: 039f85c

Merging this PR will not cause a version bump for any packages. If these changes should not result in a new version, you're good to go. If these changes should result in a version bump, you need to add a changeset.

This PR includes no changesets

When changesets are added to this PR, you'll see the packages that this PR includes changesets for and the associated semver types

Click here to learn what changesets are, and how to add one.

Click here if you're a maintainer who wants to add a changeset to this PR

[Copilot]

Pull Request Overview

This PR fixes a dependency issue in the useExternalStoreRuntime hook by adding the store parameter to the useEffect dependency array. This ensures that runtime.setAdapter is properly re-executed when the store reference changes, preventing potential stale closure issues.

• Added store to the useEffect dependency array in useExternalStoreRuntime

---

_{Tip: Customize your code reviews with copilot-instructions.md. Create the file or learn how to get started.}

[greptile-apps]

_{1 file reviewed, no comments}

_{Edit Code Review Agent Settings | Greptile}

[cubic-dev-ai]

No issues found across 1 file

[claude]

Pull Request Review

Summary

This PR fixes a React hooks dependency issue in useExternalStoreRuntime where the useEffect was missing dependencies.

---

Positive Aspects

1. Critical Bug Fix: The missing dependency array in the first useEffect (line 17-19) was a serious bug that violated React's Rules of Hooks. The effect would run on every render, potentially causing performance issues and stale closure bugs.

2. Correct Dependencies: Adding [runtime, store] ensures the adapter is updated whenever the store reference changes.

3. Import Organization: The imports are now alphabetically sorted, which improves code consistency.

---

Issues Identified

1. Missing Changeset (Required)

This PR modifies @assistant-ui/react which is a published package (v0.11.29). According to CONTRIBUTING.md, every pull request that changes packages must include a changeset.

Action Required: Run 'pnpm changeset' to create a changeset file. This should be a patch version bump.

2. Cleanup Function Regression

The second useEffect change (lines 23-27) removes the cleanup return statement. The original code returned the cleanup function from registerModelContextProvider. The new code does not return anything, which means the cleanup will not run when the component unmounts or dependencies change.

Impact: This could lead to memory leaks or stale registrations.

Recommendation: Keep the return statement to ensure proper cleanup.

---

Additional Observations

Test Coverage: No tests were found in the external-store directory. Consider adding tests for store reference changes, proper cleanup when component unmounts, and model context registration lifecycle.

Documentation: No API surface changes, so documentation updates are not required.

---

Code Quality Assessment

Rating: Good - The main fix is correct and addresses a real bug, but the cleanup function removal needs to be addressed.

Security: No security concerns
Performance: Improved (prevents unnecessary effect runs)
Best Practices: Partially follows best practices (cleanup function issue)

---

Recommended Actions

1. Add a changeset (required for merge)
2. Fix the cleanup function in the second useEffect
3. Consider adding tests for this hook behavior

---

Verdict

The core fix for the dependency array is excellent and necessary. However, two issues must be addressed before merging:

1. Missing changeset (blocks release)
2. Removed cleanup function (potential memory leak)

Once these are resolved, this will be a solid bug fix!

[Yonom]

This commit does not change the behavior of the code. I am unable to reproduce the error you are specifying. Thank you nevertheless for the PR :)