Skip to content

Draft: failing suspense test#70

Open
lazakrisz wants to merge 2 commits into
saasquatch:mainfrom
lazakrisz:test-infinite-molecule
Open

Draft: failing suspense test#70
lazakrisz wants to merge 2 commits into
saasquatch:mainfrom
lazakrisz:test-infinite-molecule

Conversation

@lazakrisz
Copy link
Copy Markdown

@lazakrisz lazakrisz commented Jun 8, 2024

Failing test case due to never resolving suspense boundary when using async atoms within molecules.

I added the failing test which is currently being discussed in #69 , currently this is just a draft pull request.

Type of change

  • Bug fix (non-breaking change that fixes an issue)
  • New feature (non-breaking change that adds functionality)
  • Breaking change (fix or feature that would cause existing functionality to not work as expected)
  • Documentation or Development tools (readme, specs, tests, code formatting)

@loganvolkers
Copy link
Copy Markdown
Member

Thanks for the PR lazakrisz I'll take a look

@lazakrisz
Copy link
Copy Markdown
Author

Thanks for the PR lazakrisz I'll take a look

No problem, I was also thinking whether this is an issue on bunshi's side or my app's side. I did a little bit more thorough explanation on the original issue: #69 (comment)
Since then I tried finding another library / reference which would show some more integration with React's Suspense, but I was unable to find any pointers.
let me know if there's anything else I can help with 🙏 I might also try some further debugging

Copilot AI review requested due to automatic review settings November 17, 2025 22:36
@changeset-bot
Copy link
Copy Markdown

changeset-bot Bot commented Nov 17, 2025

⚠️ No Changeset found

Latest commit: b6c0d0c

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

Copy link
Copy Markdown
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull Request Overview

This PR adds a failing test case to demonstrate an issue where Suspense boundaries never resolve when using async atoms within molecules (as discussed in issue #69). The test specifically examines the behavior when ScopeProvider is positioned as a child of a Suspense boundary.

Key Changes

  • Added three new test cases for async atom behavior with Suspense boundaries
  • Added react-dom as a dev dependency
  • Added a debug test script for troubleshooting

Reviewed Changes

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

File Description
src/react/ScopeProvider.test.tsx Added three test cases examining promise atoms and Suspense boundary behavior with different component hierarchies
package.json Added react-dom dev dependency and debug test script

}
});

test("Promise atom changes from loading to hasData and contains sattled data", async () => {
Copy link

Copilot AI Nov 17, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Corrected spelling of 'sattled' to 'settled'.

Suggested change
test("Promise atom changes from loading to hasData and contains sattled data", async () => {
test("Promise atom changes from loading to hasData and contains settled data", async () => {

Copilot uses AI. Check for mistakes.
use(SuspenseScope);
lifecycle.connect();

// dummy promise atom without lodable which should trigger suspense
Copy link

Copilot AI Nov 17, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Corrected spelling of 'lodable' to 'loadable'.

Suggested change
// dummy promise atom without lodable which should trigger suspense
// dummy promise atom without loadable which should trigger suspense

Copilot uses AI. Check for mistakes.
use(SuspenseScope);
lifecycle.connect();

// dummy promise atom without lodable which should trigger suspense
Copy link

Copilot AI Nov 17, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Corrected spelling of 'lodable' to 'loadable'.

Suggested change
// dummy promise atom without lodable which should trigger suspense
// dummy promise atom without loadable which should trigger suspense

Copilot uses AI. Check for mistakes.
act,
waitFor,
renderHook,
getByText,
Copy link

Copilot AI Nov 17, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The imported getByText function is unused in this file. Consider removing it to keep imports clean.

Suggested change
getByText,

Copilot uses AI. Check for mistakes.
useSetAtom,
} from "jotai";
import React, { ReactNode, useContext, useEffect } from "react";
import { atomWithObservable, loadable } from "jotai/vanilla/utils";
Copy link

Copilot AI Nov 17, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The imported atomWithObservable function is unused in this file. Consider removing it to keep imports clean.

Suggested change
import { atomWithObservable, loadable } from "jotai/vanilla/utils";
import { loadable } from "jotai/vanilla/utils";

Copilot uses AI. Check for mistakes.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Development

Successfully merging this pull request may close these issues.

3 participants