Skip to content

Refactor useObservable to handle deeply nested values#4

Open
hrastnik wants to merge 4 commits intocoolsoftwaretyler:mainfrom
hrastnik:refactor-use-observable
Open

Refactor useObservable to handle deeply nested values#4
hrastnik wants to merge 4 commits intocoolsoftwaretyler:mainfrom
hrastnik:refactor-use-observable

Conversation

@hrastnik
Copy link

I rewrote the implementation to use recursion to track which properties have been accessed.

Then, in a useLayoutEffect it replays the property accesses it tracked during render in a reaction so it knows when to re-render

useLayoutEffect(() => {
const disposer = reaction(
function expression() {
//// Uncomment this to see what paths are being accessed in the render

Choose a reason for hiding this comment

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

suggestion: let's change this to a logging function that we can configure with an optional argument to useObservable. Maybe just like useObservable<T extends IStateTreeNode>(model: T, debug?: boolean), and then if debug is true, we run these logs?

Choose a reason for hiding this comment

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

I've got that locally, will push up tomorrow.

@coolsoftwaretyler
Copy link
Owner

Thanks @hrastnik! This is awesome.

I was able to confirm the getter with arguments (lowercase()) is working with this, now.

There are quite a few TypeScript errors here, and we've lost the types coming back from useObservable. Do you want to fix those up? I can handle that, but figured I'd give you the chance before I start pushing to your branch.

@hrastnik
Copy link
Author

Feel free to push to the branch. I won't have much time until the weekend to work on this.

@coolsoftwaretyler
Copy link
Owner

Will do! I'll make a few modifications here and let you know when I do so you can pull things down.

Thanks again!

@coolsoftwaretyler
Copy link
Owner

Just quickly pushed up some of the code to demo the lowerCase function on each of the example pages.

@coolsoftwaretyler
Copy link
Owner

I'll try to push up a few more ideas tomorrow, although I'm about to do a cross country move, so I may be a little slow to respond until late next week.

@coolsoftwaretyler
Copy link
Owner

In 3c2e29e, I fixed up the TypeScript errors and the inference, also added a new Class to hold our possible paths/args.

Then in b27c77f, I added a view with actual parameters to test the type checking on those, and used it in the example routes.

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.

2 participants