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

Adjust logic responsible for cloning elements #19

Open
wants to merge 2 commits into
base: main
Choose a base branch
from

Conversation

pawelblaszczyk5
Copy link

@pawelblaszczyk5 pawelblaszczyk5 commented Nov 5, 2024

Hello, I have another thing that I stumbled upon while playing with Restyle 😃

Current check verifies only whether something is string and in that cases it avoids cloning. However, besides strings there're many things that are supported as renderable node in React. That makes code like this throw error:

<span css={{}}>{5}</span>
<span css={{}}>{5n}</span>
<span css={{}}>{true}</span>

And like this with React 19 (context, promises, generators, async iterables are renderable nodes now):

<span css={{}}>{Promise.resolve("test")}</span>
<span css={{}}>{React.createContext("anything")}</span>
<span css={{}}>{getAsyncIterator()}</span>
<span css={{}}>{getGenerator()}</span>

This is the error:

image

I've tried to investigate React source code what's the best way to differentiate whether that's something "renderable" by React, but that logic is pretty convoluted.

I've came to idea to do it the other way and differentiate whether something is clonable, because that function logic is much easier. I implemented it as a simple check whether something is object and whether has type property. This is still not perfect and I'm open for better ideas, but I feel like it's much better than the current one and will only fail in cases where built-in JSX transform would also fail (but at different place). I've also looked through and it seems like the type isn't some implementation details but it's official API.

I also have another idea, maybe it’s worth using react-is here?

My current workaround is to just add "space" after the node, so it's handled as array 😅

<span css={{}}>{5} </span>
<span css={{}}>{5n} </span>
<span css={{}}>{true} </span>

Also when applying my patch as change in my project and test-driving it, I noticed one more thing (see difference between 1st and 2nd commit).

Stuff like this still wasn't properly rendered because of truthy check:

<span css={{}}>{0}</span>
<span css={{}}>{0n}</span>

So I adjusted also the truthiness check and I try to mostly give the job of avoiding falsy values back to React

Copy link

vercel bot commented Nov 5, 2024

The latest updates on your projects. Learn more about Vercel for Git ↗︎

Name Status Preview Comments Updated (UTC)
restyle ✅ Ready (Inspect) Visit Preview 💬 Add feedback Nov 5, 2024 7:09pm

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.

1 participant