docker.mdx: use --omit=dev for npm over deprecated --production

[corneliusroemer]

Description (required)

This fixes npm warnings that I get using the current suggested Dockerfile, see warnings:

[linux/arm64 build-deps 1/1] 
RUN npm install --production=false
npm WARN config production Use `--omit=dev` instead.
RUN npm install --production
npm WARN config production Use `--omit=dev` instead.

Related issues & labels (optional)

• Closes #
• Suggested label:

[vercel]

The latest updates on your projects. Learn more about Vercel for Git ↗︎

Name	Status	Preview	Comments	Updated (UTC)
docs	✅ Ready (Inspect)	Visit Preview	💬 Add feedback	May 14, 2024 0:55am
docs-i18n	🔄 Building (Inspect)	Visit Preview	💬 Add feedback	May 14, 2024 0:55am

[astrobot-houston]

Hello! Thank you for opening your first PR to Astro’s Docs! 🎉

Here’s what will happen next:

1. Our GitHub bots will run to check your changes.
   If they spot any broken links you will see some error messages on this PR.
   Don’t hesitate to ask any questions if you’re not sure what these mean!

2. In a few minutes, you’ll be able to see a preview of your changes on Vercel 🥳

3. One or more of our maintainers will take a look and may ask you to make changes.
   We try to be responsive, but don’t worry if this takes a few days.

[astrobot-houston]

Lunaria Status Overview

🌕 This pull request will trigger status changes.

Learn more

By default, every PR changing files present in the Lunaria configuration's files property will be considered and trigger status changes accordingly.

You can change this by adding one of the keywords present in the ignoreKeywords property in your Lunaria configuration file in the PR's title (ignoring all files) or by including a tracker directive in the merged commit's description.

Tracked Files

Locale	File	Note
en	recipes/docker.mdx	Source changed, localizations will be marked as outdated.

Warnings reference

Icon	Description
🔄️	The source for this localization has been updated since the creation of this pull request, make sure all changes in the source have been applied.

[adaliszk]

Niptick: While it works, I would instead specify the intended folders to copy. This solution heavily relies on .dockerignore being correct and not allowing anything unwanted to be copied.

[adaliszk]

Suggestion/Nitpick: While . works fine, perhaps /app/ would convey more information without relying on the previous state of the WORKDIR.

[wassfila]

it's different working styles, I guess the doc purpose is only to convey examples, it's hard to cover all use cases, and sometimes people use them in different context, some even develop with live reload under docker. relying on workdir also prevents changing it everywhere in case of rename.

[adaliszk]

I would still advise against using COPY . . to avoid beginner mistakes. With HRM you would mount the files, and that is fine, but when you COPY, you do copy files into your image! You can very easily make a mistake and blow up your image size and even leak sensitive information with things like .env.

[adaliszk]

Suggestion: Instead of install, it would be safer to use ci. At this stage, installing is essentially a clean install, but if someone copies this into a different context, they might face some unintended behaviors.

Suggested change

	RUN npm install
	RUN npm ci

[sarah11918]

I will keep this consistent with the original instruction, just removing the flag. If there are changes to be made to the recipe otherwise, we can always revisit!

[adaliszk]

Suggestion/Nitpick: While ./ works fine, perhaps /app/ would convey more information without relying on the previous state of the WORKDIR.

[wassfila]

both are ok I think

[wassfila]

impact of changes is low, changes are strictly equivalent but newer notation so fancy future proof.

[wassfila]

both are ok I think

[wassfila]

it's different working styles, I guess the doc purpose is only to convey examples, it's hard to cover all use cases, and sometimes people use them in different context, some even develop with live reload under docker. relying on workdir also prevents changing it everywhere in case of rename.

[sarah11918]

Thank you everyone for discussing this so thoughtfully and thoroughly!

(And reminder, at this point it's only the submitted change that we're considering. If there's other text on the page someone feels should be changed, that's out of the scope of this proposed change and a separate PR can be made.)

[sarah11918]

Thank you for taking the time to contribute back the fix that worked for you @corneliusroemer ! We appreciate your efforts, and are happy to welcome you to Team Docs! 🥳