Skip to content

spike: some experiments on use(obserable) - very rough #2

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

Closed
wants to merge 5 commits into from

Conversation

coolsoftwaretyler
Copy link
Owner

Extremely rough, actual implementation code that isn't even close to working.

Run test:

yarn install
yarn test packages/react-reconciler/src/__tests__/ReactUseObservable-test.js

@coolsoftwaretyler
Copy link
Owner Author

@coolsoftwaretyler
Copy link
Owner Author

coolsoftwaretyler commented Sep 8, 2024

The diff looks bigger than it is because my editor formatted it, but you'll wanna look for the use<T function, or readObservable for new code

@@ -1191,13 +1168,29 @@ function use<T>(usable: Usable<T>): T {
} else if (usable.$$typeof === REACT_CONTEXT_TYPE) {
const context: ReactContext<T> = (usable: any);
return readContext(context);
} else if (usable.$$typeof === 'REACT_OBSERVABLE_TYPE') {
Copy link
Owner Author

Choose a reason for hiding this comment

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

This is the beginning of the relevant changes

@coolsoftwaretyler
Copy link
Owner Author

coolsoftwaretyler commented Sep 8, 2024

In 7c40b75, I just started logging the name value in Sync, and the output when you run the test contains:

Running tests for default (experimental)...
  console.log
    name First

      at Sync (packages/react-reconciler/src/__tests__/ReactUseObservable-test.js:97:15)

  console.log
    name Second

      at Sync (packages/react-reconciler/src/__tests__/ReactUseObservable-test.js:97:15)

I also changed the names to First and Second for more clarity.

So it seems like the observability is working, but I'm making a silly mistake in the test here.

@coolsoftwaretyler
Copy link
Owner Author

Hey @rcharmeyer - can you say more about what you mean here?

@rcharmeyer
Copy link

Hey @rcharmeyer - can you say more about what you mean here?

Nevermind, I was just hallucinating a use case that doesn't exist.

@coolsoftwaretyler
Copy link
Owner Author

Hey @rcharmeyer - can you say more about what you mean here?

Nevermind, I was just hallucinating a use case that doesn't exist.

Haha I 100% understand. The last few days have been full of that for me. Thanks for taking a look anyway! If you know more about React internals than me, I'd appreciate any insights you have.

@coolsoftwaretyler
Copy link
Owner Author

Heyo, I think I got it.

Since this hacky code doesn't know anything about memoizedProps and React doesn't know about it, the reconciler was bailing out early. That's why the console logs ran (ahead of the actual reconciliation step), but the output wouldn't update in test rendered output or when I was demoing in a real browser.

So now the callback calls fiber.memoizedProps = null which strikes me as 100% not the way this should be handled.

But, it makes this demo minimally functional. Run yarn test packages/react-reconciler/src/__tests__/ReactUseObservable-test.js and see that updating the observed value gets it to update the rendered output.

Copy link

This pull request has been automatically marked as stale. If this pull request is still relevant, please leave any comment (for example, "bump"), and we'll keep it open. We are sorry that we haven't been able to prioritize reviewing it yet. Your contribution is very much appreciated.

Copy link

Closing this pull request after a prolonged period of inactivity. If this issue is still present in the latest release, please ask for this pull request to be reopened. Thank you!

@github-actions github-actions bot closed this Mar 18, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants