Style issues / feedback

shuding commented 2 years ago

Here's my before & after in case you find it helpful (playground link).

I ended up starting mostly from scratch since there were so many small issues (definitely not a knock; this library is amazing).

Some of the issues I ran into include:

~~flexbox gap property unsupported~~
vmin, vmax, vw, vh units silently unsupported
specifying fontSize in em didn't seem to take effect
the div must explicitly declare display: flex error came up a lot and is pretty annoying, though I understand the intent behind it since it differs from the normal HTML spec
backgroundSize: cover not supported
position: absolute doesn't seem to respect it's nearest relative ancestor; e.g,. it always is relative to the document root
I had issues getting the local next.js version to display the same as the playground, presumably since they're both using slightly different versions of Satori and the lib is new + updating frequently. Some things like filter: blur(8px) + transform: scale(1.1) would work on the playground but not with the latest version of next.js. Not sure there's much that can be done with these sorts of things aside from just maturing the libs, but I wanted to at least call it out.

None of these issues is a big deal; the real pain is that they're all silent, and the only way to figure out what works & what doesn't is through trial & error.

I know some of the compatibility issues are covered in the readme already and this list goes outside the scope of this issue, but I'm just giving feedback on my experience and hope you find it helpful.

Originally posted by @transitive-bullshit in https://github.com/vercel/satori/issues/41#issuecomment-1279556004

gigantino commented 2 years ago

I can confirm most of those issues, the one I've been missing the most though is surely the "gap" flex property, reason why I came here in the first place.

transitive-bullshit commented 2 years ago

It looks like gap support should be included in the next release of yoga-layout https://github.com/facebook/yoga/pull/1116, though yoga doesn't seem to release updates very often.

shuding commented 2 years ago

Yes, gap was landed in React Native too. It will take some effort to port latest Yoga to asm.js + WASM, and then test them in Satori.

Regarding vw, vh, I think they're already supported. Please share if you have a failing case, thanks!

vmin and vmax should be quiet easy. Since it's a static card, it shouldn't be hard to work around them as well. Same for em in fontSize.

NickGerleman commented 2 years ago

It looks like gap support should be included in the next release of yoga-layout facebook/yoga#1116, though yoga doesn't seem to release updates very often.

Doing a new release of Yoga is on my radar to happen soon, but this will likely only target Android and Apple platforms for now (packages on Maven Central and Cocoapods). The other supported platforms are not yet in a healthy state (and Apple really isn't either, but I think won't be too much work).

I've honestly not been sure what to do with the NodeJS bindings. ~embind~nbind, which was being used before to allow both NodeJS bindings (via V8 FFI), and browser bindings (via emscripten) is a dead-end, unsupported past Node 10 (though you can hack it to work with Node 12). So things like yoga-layout-prebuilt redistribute the browser build without that bit.

Jettisoning the NodeJS support and moving to a binding system meant just for asm.js/WASM would bring a lot of the goodness we had before, into something that can be used now. I haven't looked closely enough on how involved this is, but contributions there would be welcome/accepted (and relatively low risk since I don't think Meta uses these bindings for anything but the Yoga website). The other outcome I could see is that somoene using these bindings in production takes ownership of supporting them (e.g. how FlexLayout did for Yoga Swift bindings), and then we would remove the crufty built-in bindings.

transitive-bullshit commented 2 years ago

I've honestly not been sure what to do with the NodeJS bindings. embind, which was being used before to allow both NodeJS bindings (via V8 FFI), and browser bindings (via emscripten) is a dead-end

I think you mean nbind is the dead-end, which is what Yoga's repo currently uses, right? embind looks to be the official Emscripten solution.

It looks like much of the hard work of porting Yoga from nbind to embind has been done by https://github.com/pinqy520/yoga-layout-wasm and https://github.com/shuding/yoga-wasm-web, though the underlying Yoga versions are out-of-date.

It looks like the most widely used NPM Yoga package is yoga-layout-prebuilt as you mentioned, which is still using nbind. I think @shuding took care to maintain parity between this package's public API and underlying yoga version with his fork in order to also adhere to @types/yoga-layout so the two distros would be interchangeable. @shuding let me know if I'm wrong in any of my conclusions; I'm just digging around a bit.

Regardless, it would be great to have a more up-to-date official solution available on the NPM ecosystem. This probably isn't the best place / issue for this discussion, however. @NickGerleman is there a better place to follow-up on this topic? Thanks!

NickGerleman commented 2 years ago

Sorry, yes nbind is what I meant to refer to.

The issue with nbind isn't the code it generates, but that the package is hooked with a build which no longer works on supported versions of Node. So yoga-layout-prebuilt does not package nbind, but is just a snapshot of its asm.js build.

Yoga's website build uses Node 12 + resolutions into unpublished nbind commits, and we have a working build. So the official Yoga repo could publish the equivalent of yoga-layout-prebuilt in sync with its sources, but we are buying time until we replace it with another binding solution, or try to adopt one from a fork. I think it would also mean dropping the Node-specific native bindings.

https://github.com/facebook/yoga/issues/979 is probably the best place for discussion.

gfox1984 commented 2 years ago

Regarding vw, vh, I think they're already supported. Please share if you have a failing case, thanks!

vh and vw don't seem to work for font-size, as least. I'm looking for a way to make the font grow or shrink based on the width and height passed to santori. vh and vw seemed the obvious choice to me... but maybe you have another suggestion?

vercel / satori

Style issues / feedback #217