lukejacksonn / oceanwind

Compiles tailwind shorthand into css at runtime. Succeeded by Twind.
https://twind.dev
264 stars 12 forks source link

feat - Support SSR #7

Closed bebraw closed 3 years ago

bebraw commented 3 years ago

I had to add basic test setup to verify it works. I also changed the project to use a newer version of Otion (now a dep).

Given the project is loosely coupled with Otion, maybe one option would be to treat it as a peer dependency and let the user manage the version. Feel free to refactor as you prefer.

Closes #1. Closes #6.

lukejacksonn commented 3 years ago

This looks great.. thanks for trying it out!

One thing I'm curious about however, is wether we can get away with supporting SSR without adding more dev dependencies to this project itself. It seems like the only change that needs to be made the the library code itself is this line:

export { setup, hydrate } from 'otion';

Along with the SSR example section you have added to the README (which is 💯 thank you).

I assume most of the other dependencies are for the jest test suite?

Regards the importing of otion, I wanted to maintain a build free, esm only setup. When I tried updating to the latest version of otion, I found it had references to process.env in the dist files, so I had to fork/vendor it and remove those statements. I also noticed it was a little larger.

bebraw commented 3 years ago

One thing I'm curious about however, is wether we can get away with supporting SSR without adding more dev dependencies to this project itself. It seems like the only change that needs to be made the the library code itself is this line:

I tried using the provided version of Otion first but it led to a crash within the fork. I imagine they've fixed something since and that's why using their distribution works. Their fixes would also explain why their dist is now larger than before.

I assume most of the other dependencies are for the jest test suite?

Yes, I set up Babel and Jest so that it's possible to run the test suite while using a modern syntax. Likely the setup can be refined further or maybe you want to port the project to TS eventually. 😄

I would also add Prettier to get consistent formatting.

Regards the importing of otion, I wanted to maintain a build free, esm only setup. When I tried updating to the latest version of otion, I found it had references to process.env in the dist files, so I had to fork/vendor it and remove those statements. I also noticed it was a little larger.

Is the point to have something that's easy to host? Can the process.env issue be fixed upstream?


I'll experiment with the solution in my current work next. The idea is to integrate it to a Deno environment. It's going to be interesting to see how consuming the package will work there. Likely I have to go through jspm or so. 😄

lukejacksonn commented 3 years ago

Ok thanks for summarising. Some actionable takeaways here then:

Is the point to have something that's easy to host? Can the process.env issue be fixed upstream?

In short yes. But really the mantra here is that this is a browser first module. That is, intended to be used in web applications running in a browser environment; contradictory to the more typical node first approach most module (which causes process.env like issues if you aren't using a bundler in dev). I like to be able to develop these kind of modules without a build step at all. Everything is static. When files change the local dev server is reloaded, the page imports the module and app tests, runs them and tells me if I broke something or not.

This probably seems backwards to most people but it has worked so far and I like the simplicity so I'd like to keep it like this if possible! That said, if you know of a lightweight testing framework that doesn't require babel then I'd certainly be interested in trying it out.

I'd love to hear more of your thoughts on this and interested to see what your next experiments yield 🚀

bebraw commented 3 years ago

Add prettier for consistent formatting (might add it to the package.json to prevent drowning in config files)

Ok, will do.

Try vendor the latest version of otion to see if it mitigates the crashing

Ok, I'll try that.

See if there is a way to test SSR reliably without jest

Would tape work? Maybe there's some light way to handle the imports without Babel. This likely depends on the version of Node you want to support. If Node 12 is acceptable, then --experimental-modules would do the trick.

lukejacksonn commented 3 years ago

Node 12 and above support is fine by me. Given that it is only one of these kind of node test and all we are really doing there is checking expect(styleTag).toBeTruthy() then I wonder if we we can get away without any "suite" runner.

I guess to run that test, we would need otion to be installed as a node module really, unless we vendor all of otion/server which I admit is not ideal. But let's see what happens. I will take a stab at it too, based off of this fork.

bebraw commented 3 years ago

Node 12 and above support is fine by me. Given that it is only one of these kind of node test and all we are really doing there is checking expect(styleTag).toBeTruthy() then I wonder if we we can get away without any "suite" runner.

Yeah, basic Node assert should be enough. I'll keep it as simple as possible.

bebraw commented 3 years ago

@lukejacksonn I did the following changes:

What remains to be solved is how to distribute otion/server. Maybe there could be oceanwind/server and we simply copy their server bundle there to consume from this package. If we go this way, then we can drop the development dependency on otion as well.

lukejacksonn commented 3 years ago

Awesome, this looks much more like it. I'll check it out properly later, see if there is anything more we can do. Maybe Jovi from #1 has some thoughts too?

bebraw commented 3 years ago

@lukejacksonn I thought about the server bit. Since we want to avoid pulling it to oceanwind itself as that would bloat its size, a good option would be to publish oceanwind-server adjacent to it. You could even go with @oceanwind/core and @oceanwind/server to get a nice namespace.

bebraw commented 3 years ago

I've been testing the work through Deno. It looks like we have to patch Otion like this:

const isDeno = window?.Deno;
const isBrowser = isDeno ? false : typeof window !== "undefined";
const isDev = isDeno ? false : process.env.NODE_ENV !== "production";

That should go where they have the process.env bit and likely you want to tune this further. Maybe isDev should be set to false always. Their browser check relies on window and that's problematic as it's defined in Deno. That's why I altered the check to check for Deno within window if it exists.

lukejacksonn commented 3 years ago

Wow man, you have been very busy. I have too but with some other things. Not ignoring this, just got a lot on my plate. It's hard to pick out exactly what has changed now since the prettier config has changed.

Could we add perhaps add { singleQuote: true } to get rid of some noise for now?

bebraw commented 3 years ago

Could we add perhaps add { singleQuote: true } to get rid of some noise for now?

Ok, done.

Feel free to tune as needed. I guess the biggest question is how to handle distribution of the server bits separately but I outlined a couple of ways above.

I still have experimentation to do with the SSR bits as now I've found some issues to debug. The basic flow works but not all classes get transformed yet. 👍