elmarsto / sbstr8

A nearly-unstyled, hackable, ultra-modern, ultra-clean scaffold for rich, interactive storytelling, journalism and blogging. Fork this repo and get to work.
https://sbstr8.lizmars.net
GNU General Public License v3.0
4 stars 0 forks source link

Theme support #11

Closed elmarsto closed 1 year ago

elmarsto commented 1 year ago

Aside from what I'll call 'Standard', which is the ultra-minimal theme that ships with sbstr8, there should be a simple mechanism for adding and overriding the theme.

I've done a lot of work to make sure that most elements are addressable from global css - the names are extraordinarily regular, and every component (and every major part of every component) has at least one CSS class that can pinpoint it. I've also made sure that there are optional props for dependency injection of components taht use other components; for example, Feature takes a linkComponent prop, so you can provide your own replacement for Link.

But that's not enough.

take, for example, the @/sbstr8/link element. It should be totally possible to override this, and sbstr8 components should 'just know' to use it.

I attempted to build something like this with the registry at @/sbstr8/index, but there's a chicken-egg problem: if I refer to this structure elsewhere in the code, then that means that whatever component I mention the structure within cannot itself be in the registry, or it would become its own dependency.

This is a surprisingly thorny problem and I don't have a good solution yet. I'm thinking that dynamic imports might have the solution, but that will require R&D to prove, and also, I worry about a (possible? actual?) hit on my tooling, e.g not sure how Typescript's language server will understand props on a dynamically imported component.

Moreover, a lot of the clever ideas I have (stuffing a list of components into context, for example) don't play nice with the server component paradigm, it's use 'client'; and weak SEO all the way once you mess with React contexts.

Other possible solutions: a WeakMap? Redux??? Something basic that I'm missing?

elmarsto commented 1 year ago

What's bothering my brain is that this is basically the problem that OO inheritance was supposed to solve; but inheritance in React JS is an antipattern; so now I need to go figure out a solution that is basically inheritance but without inheritance. I can live with this, but it still annoys me, in the same way that building a database table to store HTML on a website bothers me (e.g. WordPress). Like, this is precisely the sort of do-over problem pinpointed in https://www.devever.net/~hl/webappcoupling and I take to be one of the underlying feeling-motivations behind this project -- that we keep needlessly reinventing the wheel, one level of abstraction higher than last time, with insufficient gain-of-function as compensation for the complexity.

elmarsto commented 1 year ago

Some R&D later:

First, Zustand doesn't work with Nextjs 13 right now, so fsck that: https://github.com/pmndrs/zustand/issues/1395

Second, the correct solution is probably to use TSyringe https://github.com/microsoft/tsyringe

I found this via https://himynameistim.com/blog/dependency-injection-with-nextjs-and-typescript

E.g. for Feature -> linkComponent provided? use that -> resolve with TSyringe -> if that fails, use Link

elmarsto commented 1 year ago

Actually I also like the graphql resolver pattern demonstrated in https://himynameistim.com/blog/dependency-injection-with-nextjs-and-typescript. Now also a work item here: #16

elmarsto commented 1 year ago

WIP https://github.com/elmarsto/sbstr8/tree/elmarsto/11-theme-support

elmarsto commented 1 year ago

Just lost an evening to this.

First of all, tsyringe is not well supported by its developers. The issues queue is mostly 'awaiting triage', and there are vital pull requests which have not been merged for literal years. Second, there's this problem with the new /app structure in next.js, which we use: https://github.com/vercel/next.js/issues/49850

Tl;dr the special circumstances concerning import 'reflect-metadata' (namely, that it can only be done once) does not play well with the fact that _app.tsx is now no longer a thing: https://github.com/vercel/next.js/issues/49850.

In short, the winning approach is actually just a good old WeakMap.

elmarsto commented 1 year ago

POC achieved. Preparing PR.

elmarsto commented 1 year ago

Winning strategy: