search

tw-in-js / use-twind-with

Twind integration packages for frameworks & libraries with examples
MIT License
68 stars 17 forks source link

feat: adding initial @twind/solid support #5

Closed amoutonbrady closed 3 years ago

amoutonbrady commented 3 years ago

Hey,

Following our discussion on twitter, here's the initial work for @twind/solid.

Few thoughts:

Again, thanks for your hard work, I really enjoy the way this project is going. You rock 🤘

sastan commented 3 years ago

Whoa! That looks really good already. I take some time tomorrow to try it and get it published! Thank you very much.

sastan commented 3 years ago

Sorry... I was busy with the typescript plugin release. I try to take a look tomorrow 🙏🏾

amoutonbrady commented 3 years ago

Ah, don't worry! There's really no rush. In fact, after a discussion with @ryansolid, I might revisit the way it's being delivered... cjs shouldn't be needed. I might mirror what you did with the other packages and add and umd build.

That'll have to wait tomorrow (gmt + 1 here).

edit: I didn't have the time today unfortunately. Hopefully I'll revisit it tomorrow :crossed_fingers:

sastan commented 3 years ago

This looks really solid...

Following our discussion on twitter, here's the initial work for @twind/solid.

Great you did it. Thank you for taking the time. Really appreciated.

Few thoughts:

  • Due to the way we handle JSX in Solid (we make a deep analysis and compile it down to minimal dom instructions, similar to Svelte), we can't really benefit ESBuild and as a direct consequence your library distilt. That being said, I really liked how compact and smooth distilt is and took on a similar approach with rollup. I abstracted all the config in a wrapper.

👍🏾

  • At the moment the preset build an esm version, a cjs version and a jsx version (this one is mostly used by us) + typings. None of those are minified as most CDN (skypack, jsdelivr, etc.) do it for us and Solid is not really used without a bundler due to its compilation requirements. In fact, after a discussion with @ryansolid, I might revisit the way it's being delivered... cjs shouldn't be needed. I might mirror what you did with the other packages and add and umd build.

Maybe cjs for SSR. Is that something in solid?

  • The test are the exact same as the @twind/react one (using SSR rendering and string comparison). I did however use a different environment setup to get it working with uvu, using jiti with a custom transform function.

👍🏾

  • This first iteration only support styled API. The custom setup one with adds tw etc. to Preact is something I'm not entirely sure is doable with Solid due to the fact that we have no virtual DOM.

Yeah that preact thing is quite unique and not a requirement. People can always choose to use the shim.

  • I kept the code as minimal as possible while maintaining readability. I can go into more details of the implementation here or in the comments of the code if needed.

It looks really good 👍🏾

  • I based most of the work on @twind/preact and @twind/react. I'm not entirely sure what is expected in term of features, dist formats, etc..

You all good!

Again, thanks for your hard work, I really enjoy the way this project is going. You rock 🤘

❤️

ryansolid commented 3 years ago

Maybe cjs for SSR. Is that something in solid?

Yes it just requires different compilation output so generally in all cases I recommend using the "solid" field which uses the source since there are a number of different compiled outputs (client-only, client+hydration, ssr+hydration, ssr-nohydration, etc..). The esm is mostly just for CDN's and no build setups where we assume client-only.

sastan commented 3 years ago

Could we support static extraction in solid, maybe even the shim?

ryansolid commented 3 years ago

Looks like it should work for Solid's synchronous rendering. renderToString works like your example.

Async is more of a challenge. We have a renderToStringAsync that returns a promise and resolves Suspense on the server. It would have to worry about concurrent requests coming in while requests are completing. I've solved this by using Context instead of a global. This might eventually be something you look into anyway for when similar features land in React. I've also solved this by spinning up separate processes but have moved away from that as of late as I made async rendering thread safe in Solid recently.

Streaming is a whole other thing I'd probably need to come up with a different sort of flush as you go solution, well beyond the scope of what we are likely doing here.

I've been doing a lot of work around creating architecture that automatically handles parallelizing loading around code splitting so data/code don't block each other. I'm trying to think if CSS could apply here. Some way of hoisting it the same way one does with say GraphQL fragments. An area that needs more consideration.

sastan commented 3 years ago

We have these (or not in case of streaming) SSR to support frameworks. It's more like we can choose from those examples the one we need for a specific framework.

Looks like it should work for Solid's synchronous rendering. renderToString works like your example.

👍🏾

Async is more of a challenge. We have a renderToStringAsync that returns a promise and resolves Suspense on the server. It would have to worry about concurrent requests coming in while requests are completing. I've solved this by using Context instead of a global. This might eventually be something you look into anyway for when similar features land in React. I've also solved this by spinning up separate processes but have moved away from that as of late as I made async rendering thread safe in Solid recently.

Twind async sheet uses async_hooks under the hood. Twinds SSR wrapper for solid must ensure to create a new "context" (like Promise.resolve().then()) and we should be good to go.

Streaming is a whole other thing I'd probably need to come up with a different sort of flush as you go solution, well beyond the scope of what we are likely doing here.

Not needed if not supported in solid.

I've been doing a lot of work around creating architecture that automatically handles parallelizing loading around code splitting so data/code don't block each other. I'm trying to think if CSS could apply here. Some way of hoisting it the same way one does with say GraphQL fragments. An area that needs more consideration.

That sounds interesting. I'm not sure if we would need that. The SSR is only used to collect the initial CSS. After rendering/hydration twind will generate CSS just-in-time in the browser.

ryansolid commented 3 years ago

Yeah I'm probably not explaining this very well and will have to look closer at the solution you posted. But Solid's async rendering does work as it goes over several frames. So if two requests come in their execution could be weaved together. Like:

A. start
B. start
A. suspense1 resolves render some more
B. suspense 1 resolves render some more
B. finishes, and writes to it's head, and sends finished HTML.
A. suspense2 resolves renders some more.
A. finishes and writes to it's head, sends finished HTML

Like it does the same fetch as you render that the browser does. So components are sometimes hit for the first time on a different microtask, and it finishes on a different task. I do have a context I pause and resume here that I could probably tap into. I was thinking I'd need something like a Virtual Sheet per render. Just not sure how to switch them.

Solid does support streaming but I haven't figured out what the best practices are for things that need to write to the head after. It is different than React or Vue which just use this chunk rather than a means to do true out of order unblocking SSR. It's more like the experimental stuff React is doing with Server Components. That being said Solid's current approach streams only data after first render so I think the synchronous solution will just work for now. So that's cool and probably make a hot demo.

sastan commented 3 years ago

It is my understanding that suspense is based on throwing promises. That should work fine with async_hooks and we do use the Promise execution tracking.

ryansolid commented 3 years ago

Oh...., I see. Yeah I wasn't aware this was a thing. And that makes a lot of sense. It's pretty cool. I kinda want to try it to see it in action. I do understand the performance consideration now. Thanks for the info.

amoutonbrady commented 3 years ago

I've had a bit of a rough week and couldn't follow through this convo. only catching up now..

Looks like you guys had quite a chat! @sastan is there anything you'd want me to take a look at? Maybe adding tests or example regarding that SSR stuff you were mentioning earlier, though I'm already using the (sync) SSR capabilities of Solid and Twind to make the tests.

sastan commented 3 years ago

It is all good. If you are happy – I would merge. The only thing I like to see, maybe in the future, is a helper for the SSR. Most users get confused by the different setups, sheet.reset(). I would like to have an API that encapsulates that, maybe @twind/solid/server or something.

amoutonbrady commented 3 years ago

Alright just pushed a couple of helpers around SSR rendering, let me know if that goes in the direction you had in mind. The renderToString seems like it works pretty well. However, the renderToStringAsync seems like it getting setup late and throw during the test.

I'd love some guidance on whether I'm going in the right direction or not before investigating this a bit further :)

edit: I put everything under the same file for now but once we are happy about how it's working, I can split it into a differing file / directory

amoutonbrady commented 3 years ago

Ah, it seems like 0.16.4 broke the whole test suite somehow

I'll move that over on Discord as it's unrelated to this PR.

amoutonbrady commented 3 years ago

I split the server into its own subpackage as suggested

sastan commented 3 years ago

Ah, it seems like 0.16.4 broke the whole test suite somehow

Yeah. Sorry... Fixed that in 0.16.6 and deprecated 0.16.4 & 0.16.5.