Update env variables docs #2375
Conversation
There was a problem hiding this comment.
Nice stuff! Once we solve all these comments, I will take another look but more from the high above, to get a better grasp for where is what and what are the relations, now I was focusing more on the details. But I think the question is similar like in other PR: Do we do A.P B.P and then link from P to those, or do we do P.A and P.B and then link from A and B to those, where are parts of docs about Production and A and B are some features (check the other PR for more info).
web/docs/deployment/deployment-methods/_addExternalAuthEnvVarsReminder.md
Show resolved
Hide resolved
web/docs/deployment/deployment-methods/_building-the-web-client.md
Outdated
Show resolved
Hide resolved
| `.env.server` should not be committed to version control as it can contain secrets, while `.env.client` can be versioned as it must not contain any secrets. | ||
| By default, in the `.gitignore` file that comes with a new Wasp app, we ignore all dotenv files. | ||
|
|
||
| :::info Dotenv files |
There was a problem hiding this comment.
Where did this go, it wasn't useful?
There was a problem hiding this comment.
It didn't feel useful to me, it talks about the specific Node.js package and they are not going to use it directly. I think the env vars file format is self-explanatory.
There was a problem hiding this comment.
They might need to understand how to use it though, to for example pass JSON values and similar, we had those situations, how to quote stuff. So I thoguht it is good to introduce the concept, since we are using it as na important part of Wasp, so it is clear this is not something we made up, but instead there are more resources behind it that they can look for. Drop it if you really want, but to me it seems useful to keep it.
There was a problem hiding this comment.
I get what you mean, if people know what's underneath they can Google better.
The env variables are first parsed by Haskell and then outputted to the .env files in the server and the client apps. If we wanted to provide full information, we should first mention Haskell's dotenv package and then our logic for outputting the vars i.e. our envVarsToDotEnvContent function. And then we can mention that the variables are ingested by Node.js with the JS dotenv package.
I dislike giving this many details to users because 99% of them don't really need them and those with issues will most likely ask about it in our Discord.
There was a problem hiding this comment.
I thought that what we had was happy middle though. But again, as I said, do as you like here, I provided my perspective as hopefully a helpful thing, I dont' have much more to say than what I already said, you will makeo ok choice whatever it is.
web/docs/project/EnvVarsTable.tsx
Outdated
There was a problem hiding this comment.
Nice this is cool! I found it a bit weird that I have to construct EnvVar elements outside and then provide them as children, to me it sounded more natural to provide a list of EnvVar objects to the EnvVarsTable and then it would call .map on those and produce a bunch of EnvVar elements that show those. But ok it is quite unimportant.
There was a problem hiding this comment.
I got inspired by ShadCN's way of composing things.
There was a problem hiding this comment.
Right, but they can combine multiple things, maybe it is Item, maybe it is Separator. You instead have a single item that can go in. So no composability, but you opened space for throwing in random elements that don't make sense. Btw do they somehow protect against being given children that you are not supposed to pass? Also they might have different reasons for this kind of design, including uniformity with the rest of the design the have or whatever.
There was a problem hiding this comment.
Compare
vs.
One of them is more readable to me, so I'd like to keep the current API. It's our internal component for docs, no need to make it so strict IMHO.
There was a problem hiding this comment.
It is more readable because you used different vars! The second example has less vars, the first and second, and third and forth, have equal length of name, and they all have much shorter notes. So there is less content, it is better aligned, and on top of all that it allowed you to zoom in so that also looks better.
Finally, your editor seems to do a better job with syntax coloring in the second example, but I don't see why it couldn't do coloring well also in the first example, I just tried this in my emacs and it does the coloring properly. Maybe that <span> in that note causes you issues? That works fine for me though.
Take a look at what it looks in my emacs, when I use the same env vars:
Those look exactly the same regarding readibility to me. If anything, you could argue that the first one has a bit less noise due to less repetitive HTML tags, although I don't think it matters.
Also, while doing the comparison, you ignored my argument about the second approach (one with children) allowing for elements that you might not expect, while the first one doesn't allow that.
Keep the API as you like it, as I said in my initial comment, this is quite unimportant, but still I think this discussion is useful as a learning opportunity and didn't want it to end with wrong arguments.
There was a problem hiding this comment.
I did another round of comments. Once we complete those, I will as a final step take a higher look from above, probably run the docs and see how they read now.
As a preparation for that, what do you say about what I asked here #2375 (review) ? What is your view on the current structure, how do you feel about it?
Updates the Project and Deployment env variables docs. I've tried to deduplicate as much as possible. I've enumerate all the env variables that Wasp defines.
Closes #1894