Add documentation about environment variables

[henrycatalinismith]

Talked about documenting this in #2447. Here's a first draft!

environment-variables.md	ClientContextEnvVars	ScaffoldedEnvVars

[richardolsson]

I think this is a great addition to the docs. I'm sorry for pulling the rug out on this otherwise (almost) mergable PR but I made some changes in #2447 before merging it that are now causing merge conflicts (and perhaps worse) here.

Basically, the types for environment variables should be consistent across the pages and app router. I took one small step in that direction in ba86abc but maybe this is a good PR to make it fully consistent?

[henrycatalinismith]

Oh nice, glad you aligned these already. I think it will make adding and removing these things even easier as you'll be able to update one central type and then treat the resulting type errors as a to-do list for completely integrating the new env var into both the app router and the page router. Looks like only teeny tiny changes ahead here so with any luck won't take long if I get a few spare minutes here and there.

[henrycatalinismith]

Right then, the merge commit is 0e1a520.

At first I was going to sit and daydream about a nice general name for a shared type. Then it occurred to me that maybe this isn't something that needs to be so meticulously neutral. If the goal state is to one day have everything be on the app router then maybe just focusing on the app router a little bit in these kinds of details is best. So I kept ClientContextEnvVars, added everything to that, and imported it in scaffold() to reuse there. What do you reckon?

[henrycatalinismith]

Should probably add something about useEnv to this right? Adding a new env var is covered nicely here but your other most likely question as someone who maybe knows Next.js but is new to Zetkin is how to read them.

[henrycatalinismith]

lol I just found another EnvVars type in src/core/env/Environment.tsx. It's actually kind of cool how documenting this stuff ends up being a way to take a holistic look at it and sometimes find ways of tidying it up.

Do we want to use that instead of this new ClientContextEnvVars? The fact that it makes everything optional seems to be really convenient for writing stuff like unit tests where I see lots of places defining only the ones that matter for the individual test.

[richardolsson]

lol I just found another EnvVars type in src/core/env/Environment.tsx. It's actually kind of cool how documenting this stuff ends up being a way to take a holistic look at it and sometimes find ways of tidying it up.

Do we want to use that instead of this new ClientContextEnvVars? The fact that it makes everything optional seems to be really convenient for writing stuff like unit tests where I see lots of places defining only the ones that matter for the individual test.

Yeah that sounds like a good idea! But unless I'm thinking about this the wrong way, it will mean that you lose the benefit of being able to change the type and get the type errors as a Todo.

I guess you can change the type without making the new var optional, handle the errors, and then make it optional.

[richardolsson]

Nice! I think this is ready to merge! 💯

Let me just say again, lovely to have you back on the roster! 🙌