Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add valtio-reactive framework #6

Draft
wants to merge 1 commit into
base: main
Choose a base branch
from
Draft

Conversation

transitive-bullshit
Copy link
Owner

See pmndrs/valtio#949 and https://github.com/valtiojs/valtio-reactive

@dai-shi I was excited to add valtio-reactive to the suite to compare apples-to-apples, but the unit tests for my valtio-reactive wrapper are currently failing. I think it's because the core signal's read isn't being tracked?

@transitive-bullshit transitive-bullshit marked this pull request as draft November 22, 2024 08:44
@transitive-bullshit
Copy link
Owner Author

valtiojs/valtio-reactive#1 might fix this?

@transitive-bullshit
Copy link
Owner Author

test(`${name} | simple write`, () => {
framework.withBuild(() => {
const s = framework.signal(2);
const c = framework.computed(() => s.read() * 2);
expect(s.read()).toEqual(2);
expect(c.read()).toEqual(4);
s.write(3);
expect(s.read()).toEqual(3);
expect(c.read()).toEqual(6);
});
});
is an example of a pretty simple test that's failing (but working for the other frameworks at the moment).

@dai-shi
Copy link

dai-shi commented Nov 22, 2024

Yeah, let me merge valtiojs/valtio-reactive#1.

@overthemike
Copy link

overthemike commented Nov 22, 2024

@transitive-bullshit that same simple test you provided works with plain valtio, right? In your branch feature/minimal-issues-repro-valtio that you had pointed to in valtio #949, those expects seem to working. Trying to wrap my head around this to dive in and help where I can.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants