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

Handle putting JSX elements into observables #6

Closed
rileymiller opened this issue Sep 20, 2022 · 3 comments
Closed

Handle putting JSX elements into observables #6

rileymiller opened this issue Sep 20, 2022 · 3 comments
Assignees
Labels
bug Something isn't working

Comments

@rileymiller
Copy link

Hey! Been doing some exploring into legend and we're super excited about it!

During our initial exploration I wanted to try and convert our dialog system to use Legend since it's pretty straightforward and ran into some issues (probably due to my misunderstanding of how legend works).

I tried two approaches, the first works great but I was curious if there's a way to get approach #2 to work since it's similar to how our team has used observables in React w/ our hand-rolled solution in the past.

Approach #1 - Working
Using observable
Here's a link to the sandbox: https://codesandbox.io/s/legend-w-observable-osyjnj

Was able to import the dialogObs created through the observable fn and access the observable directly inside any child components, didn't run into any issues w/ this.

Approach #2 - Borked
Using useObservable

Link to the sandbox: https://codesandbox.io/s/legend-useobservable-uwj1cu?file=/src/App.tsx:85-98

Wanted to see if we could pass down the dialogObs directly through a line of child components via props and it's throwing a TypeError:

'getOwnPropertyDescriptor' on proxy: trap reported non-configurability for property '$$typeof' which is either non-existent or configurable in the proxy target

Is there an issue w/ the way I'm trying to use useObservable or is there something else going on?

@jmeistrich
Copy link
Contributor

Oh wow, I had not considered putting ReactNodes directly into observables. I would approach it by putting a lighter object that describes the dialog into the observable, like { title, type, info } etc... Then have your Dialog component use those details to render. Something like this: https://codesandbox.io/s/legend-w-observable-forked-o4zzxp?file=/src/App.tsx

That said though, the actual problem is when you call set, legend-state iterates through all child nodes to see if there's any listeners to notify. Since the JSX object is quite large that's not great, but the crashing problem in the useObservable version is it includes an observable in a prop.

I think the fix is to skip proxying ReactNodes and just treat them as opaque objects. I will try to get a fix into an update soon. For now I'd suggest avoiding putting ReactNodes directly into observables.

@jmeistrich jmeistrich self-assigned this Sep 21, 2022
@rileymiller
Copy link
Author

Ah gotcha, thanks for taking a look at this!

@jmeistrich jmeistrich changed the title Passing Observable as prop to a Component stored in the observable Handle putting JSX elements into observables Sep 25, 2022
@jmeistrich jmeistrich added the bug Something isn't working label Sep 25, 2022
@jmeistrich
Copy link
Contributor

jmeistrich commented Sep 26, 2022

Version 0.18 will handle this better and also show console warnings. There's some other big changes in 0.18 so we're testing it in 0.18.0@next first, but it's looking good so you can try it now if you'd like, or 0.18 should be released shortly.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

No branches or pull requests

2 participants