Deno compatibility
katywings opened this issue · 20 comments
Motivation
I am trying to make Otion compatible with Deno. As a poc I created this little transform script: https://github.com/lufrai/deno-otion/blob/master/update.ts
Details
It already looks like Otion is mostly working in Deno with the changes. Is this something you would be interested in to integrate into your Otion builds?
One thing thats still a mystery is Otions server side rendering: in my Deno tries it only works for the first request, even though I run setup with a new VirtualInjector like here on each request. The style tag is just empty on all requests after the first one 😅🙈. Since i only tried it out with Deno yet, I am unsure if this might be in node as well.
Edit: From looking at the Otion code I think the problem mentioned at the top with the style tag being empty after the first render actually also exists in node:
The injector only gets new css here: https://github.com/kripod/otion/blob/master/packages/otion/src/createInstance.ts#L183. But the setup call never deletes the insertedIdentNames from previous ssr.
Whoa, thank you for the detailed suggestion! Sure, Deno should be supported out of the box. The window → document change is an easy one to start with. After that, I think Rollup should be configured to create separate dev and prod builds, so import maps could resolve to either of them (defaulting to the development production build). Please let me know if there is a better convention I'm not being aware about.
I would appreciate if you could set up a minimal reproduction case for testing SSR, even with Node. Thank you for all the efforts put into this!
Hey @kripod
Thank you for your quick response (: i see that you already committed the window -> document change, awesome!
Regarding process.env.NODE_ENV: I actually couldnt find any clear Deno related best practice but the same topic is discussed in graphql/graphql-js#2409. Well today I will not have time to read through this though 😂.
I probably can create you a test script and proposed fix regarding ssr at wednesday/thursday ish.
Wonderful, thank you very much!
@kripod Earlier than expected: here you go :), a little codesandbox with some tests and a proposed fix in otion setup (look for katyFix :D)
https://codesandbox.io/s/modest-varahamihira-9l4gr?file=/src/index.js
P.S. if you make changes in the code on codesandbox you should reload the inline browser manually because codesandbox does some hmr and keeps the old insertedIdentNames in memory as well (when my fix is disabled ;))
Thank you for the proposed fix! Meanwhile, I've released v0.2 which should offer Deno support out of the box. Please use import maps as shown:
/* import_map.json */
{
"imports": {
"otion/dev": "https://unpkg.com/otion@0.2.0/dist-deno/bundle.dev.mjs",
"otion": "https://unpkg.com/otion@0.2.0/dist-deno/bundle.prod.min.mjs"
}
}@kripod But did you see the katyFix in my codesandbox? Thats missing in otion, its needed for ssr (independed from Deno) 😅
Yes, thank you! I’ll check it out after having lunch.
😂
You too! 😄
All these issues should be gone with the freshly-released v0.2.2. I've also added instructions about supporting Deno to the core package's readme.
@katywings I'm extremely grateful for all the information you provided. The new version wouldn't have been possible without your help. 🙌
@all-contributors please add @katywings for reporting a bug and the idea about supporting Deno.
I've put up a pull request to add @katywings! 🎉
@kripod Thanks a lot for working the Deno things out with me :). Well I am really sorry but I noticed a follow up problem in the new deno builds xD
https://unpkg.com/browse/otion@0.2.4/dist-deno/bundle.dev.d.ts
Looking at the second line import * as CSS from "csstype" Deno will actually try to resolve this import but it can't find 'csstype'.
thread 'main' panicked at 'Can't downcast ErrBox(JSError(JSError { message: "Uncaught Error: Unmapped bare specifier \"csstype\"", sour.....
If you want I can try to figure out from where the csstype thing this is coming tomorrow :).
Thank you for the feedback!
I noticed that recently and updated the Deno setup guide to suggest using Pika CDN, as it correctly resolves csstype as a dependency, based on package.json. Hopefully, other CDNs will catch up with ESM support.
@kripod Ah nice great docs you got there <3!
Pika CDN is super easy, barely an inconvinience: denoland/deno#5665
🤣
Should the current compatibility path work with otion/server as well? I'm trying to get some server-side rendering done with Otion while using Deno environment so I'm curious about this. I can open a separate issue if that's better.
I checked Pika but it looks like they've rebranded themselves as skypack.
In addition the Deno build seems to be the runtime but does it contain the server code as well? Then the question is how to consume that.
I haven't been experimenting with Deno SSR, but it should work without issues, as the code doesn't rely upon Node's globals (like process.env.NODE_ENV).
@kripod In my use case I'm doing server-side rendering with Otion and I don't see the server specific bits at dist-deno in the distribution. In addition, I use the client side through server as well.
To solve that, I had to tune your runtime code to include a more specific check like this:
const isDeno = window?.Deno;
const isBrowser = isDeno ? false : typeof window !== "undefined";
const isDev = isDeno ? false : process.env.NODE_ENV !== "production";The process.env check is problem for oceanwind as well and that's why they're using a modified version of Otion for now. I believe that process.env bit may require a more specific check so that you can use the runtime in the browser without any compilation or processing as that's the use case for Oceanwind.