build: creates a devcontainer configuration for contributors

[artis3n]

I had issues with my local version of NPM being incompatible with the package.json requirements (even though it's within the stated ^10...), and figured it would be useful to create a standard development environment configuration via a devcontainer config. It can be used by a number of 3rd party tools these days, but it primarily makes GitHub Codespaces a convenient place to get a working dev environment for this repo.

[github-actions]

	Messages
📖	✨ Thanks for your contribution to Shields, @artis3n!
📖	Thanks for contributing to our documentation. We ❤️ our documentarians!

Generated by 🚫 dangerJS against 8840230

[artis3n]

Set Node 20 b/c when I try to use the node 22 image (or locally), I can get following error:

[chris48s]

Is there a particular reason for picking this image? I suppose it doesn't make much difference for an application like this one with basically zero "system" dependencies beyond node, but my instinct would be to base this off be node:20-alpine since that is what we use in https://github.com/badges/shields/blob/master/Dockerfile

In any case, yes. We definitely want node 20

[artis3n]

vscode can generate a devcontainer which then uses Microsoft's set of "officially maintained" ones. That's where this image comes from. I can use a basic node:20-alpine as well, the devcontainer config will just need to also prep git and some other things. Given it's "the node image designed for devcontainers" managed by Microsoft, would you be fine leaving it in? Happy to change it to node:20-alpine and the subsequent other changes to the devcontainer if you'd prefer.

[chris48s]

So there's a couple of things here:

1. Can you give some more details of the problems you hit. If we're saying "you can use npm 10 locally" and that doesn't work for some reason (and it turns out that is a verified problem related to a specific version of npm) I'd like to know about that and update the engines constraint accordingly.

2. The issue with devcontainers in general is that they mix "stuff that is necessary to run the app" and "tools/extensions you like to use". I think if we were going to add something like this to this repo, what I'd want to do is provide a really minimal setup with just what you need to run the app, and then leave users to extend that with their own preferences on tools/extensions they want to use around that. I don't think Shields should be opinionated about what editor extensions contributors use, and doing so is a magnet for bikeshedding.

[artis3n]

The issue with devcontainers in general is that they mix "stuff that is necessary to run the app" and "tools/extensions you like to use". I think if we were going to add something like this to this repo, what I'd want to do is provide a really minimal setup with just what you need to run the app, and then leave users to extend that with their own preferences on tools/extensions they want to use around that. I don't think Shields should be opinionated about what editor extensions contributors use, and doing so is a magnet for bikeshedding.

Makes total sense! I included those extensions because of what the package.json does - e.g. include the prettier extension because of npm run prettier. Similarly the npm intellisense one because node. I picked the biggest downloads/most "popular" extensions for those with the intention of being unopinionated about what other extensions people might want (e.g. no markdown extension because the repo doesn't do specific auto-md formatting that all contributors should follow).

I am installing postgresql because of the CONTRIBUTING instructions that require setting up Postgres to run some of the tests. Someone still needs to start the DB and set the env var, but I preinstalled Postgres into the environment.

The github-cli feature I installed so that the gh CLI is available for folks to push PRs. git is another feature I'd normally install but it comes already in this environment so no need.

Otherwise we have a minimal config: use the Node 22 image and run npm ci on the user's behalf when an environment spins up.

Totally happy to remove the vscode.extensions section if you'd prefer no extensions be pre-selected, as well as any of the above. Just let me know where you'd like to set the threshold! Sounds like at minimum we should drop the extensions configuration.

[artis3n]

Can you give some more details of the problems you hit. If we're saying "you can use npm 10 locally" and that doesn't work for some reason (and it turns out that is a verified problem related to a specific version of npm) I'd like to know about that and update the engines constraint accordingly.

Yeah, totally. I am not sure what's up but I can't get npm ci to run on Node 22 because I get this error which seems to say the npm version is bad 🤷 Got the same error on my local machine (which was node 23 at first, then downgraded to node 22) as on this screenshot from the Node 22 devcontainer image. Haven't looked into it further.

The easiest way to reproduce would probably be to pull the mcr.microsoft.com/devcontainers/javascript-node:1-22-bookworm image, and copy over this repo's package*.json and try npm ci. Or spin up a codespace with that 22-bookworm image.

#10937 (comment)

edit: am just now seeing the screenshot says "not compatible with your version of node/npm" I read that as "npm". So it's more likely to do with incompatibility with Node 22.x? Which could be expected by you - I assumed 20 or 22 would work since there was a mention that tests must pass against both but there isn't an equivalent || in the node engine requirement string.

[chris48s]

edit: am just now seeing the screenshot says "not compatible with your version of node/npm" I read that as "npm". So it's more likely to do with incompatibility with Node 22.x

Right, yes. We're currently running on Node 20 in production so that's what we ask users to run locally https://github.com/badges/shields/blob/master/doc/TUTORIAL.md#node-npm .
We also run the tests against Node 22 in CI. I'm planning to move production to Node 22 in the near future - probably April.

I should have updated the engines declaration when we got everything passing on node 22. I've done this in #10940 That allows you to install on node 22 without specifying NPM_CONFIG_ENGINE_STRICT=false

[chris48s]

I picked the biggest downloads/most "popular" extensions

I think lets not specify any extensions here.

I am installing postgresql because of the CONTRIBUTING instructions that require setting up Postgres to run some of the tests.

Postgres is required for the integration tests (npm run test:integration) but they are not part of the main test suite. If you run npm test those tests are skipped. Most contributors can ignore them.

https://github.com/badges/shields/blob/master/CONTRIBUTING.md#tests

I think if you want to provide a working postgres as part of a devcontainer setup there's probably a bit more to do here than just install a package. If you want to do that, can you run through the process of what you need to do to get to yourself a stage where you've got the integration tests passing in that environment.

If we're going to take on the maintenance and support of this (which is what you're asking us to do here) we need to do one of the following:

• provide a fully working setup out of the box
• provide clear instructions for what you need to do to get to a fully working setup
• just drop trying to provide postgres

Tbh, I'm inclined to just keep it really really simple if we're going to take this on. I don't use devcontainers myself, so what I really don't want is to merge this and then someone opens an issue saying "I'm trying to contribute to shields and I'm stuck because [something something devcontainers]". It is just a category of support I have no appetite for.

[chris48s]

Is there a particular reason for picking this image? I suppose it doesn't make much difference for an application like this one with basically zero "system" dependencies beyond node, but my instinct would be to base this off be node:20-alpine since that is what we use in https://github.com/badges/shields/blob/master/Dockerfile

In any case, yes. We definitely want node 20

[artis3n]

Tbh, I'm inclined to just keep it really really simple if we're going to take this on. I don't use devcontainers myself, so what I really don't want is to merge this and then someone opens an issue saying "I'm trying to contribute to shields and I'm stuck because [something something devcontainers]". It is just a category of support I have no appetite for.

Absolutely, makes sense. Let me strip it to the absolute minimum that lets someone get a working environment to contribute badges.

Right, yes. We're currently running on Node 20 in production so that's what we ask users to run locally https://github.com/badges/shields/blob/master/doc/TUTORIAL.md#node-npm .

Ahh, I didn't take Node >= 20 to mean < 22. Maybe a Node ~= 20 note in the docs to illustrate users should not go above that major version?