-
-
Notifications
You must be signed in to change notification settings - Fork 1.3k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
[docs] Add warning about process.env.NODE_ENV
in production
#13869
[docs] Add warning about process.env.NODE_ENV
in production
#13869
Conversation
Deploy preview: https://deploy-preview-13869--material-ui-x.netlify.app/ Updated pages: |
Make sure to set `process.env.NODE_ENV` to `'production'` in your build process to avoid the watermark in production. | ||
Most bundlers set this environment variable automatically when building for production, but for custom setups, you might need to set it manually. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It would probably be good to expand on this or reference some docs on why
it is not a good idea to have NODE_ENV
set to anything else than production
when the code is live.
I've been to plenty of companies where NODE_ENV=stage
or NODE_ENV=prod
were used and I had to go on a crusade to convince people who didn't know about this. 🙃
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Do you have any good links we could share with our users here, @JCQuintas?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I agree that if we can make people understand that it's not some MUI specific needs but that you should always respect this convention because lots of packages relies on this infomation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@joserodolfofreitas Not really, this is a convention only... Hence there is no actual documentation from node on it.
Maybe webpack? Though it is not super complete. I would try to inform people why it is a bad idea and link to a few similar sources.
@flaviendelangle yeah, but you can't expect everyone to be reasonable about this. If we have actual documentation on the why
, then an engineer can go into their superiors and say "this pattern we are using is wrong because it can cause issues in downstream libraries, here is the docs"
.
It prevents issues on our side, if someone comes in asking "can you remove the watermark when NODE_ENV=prod?"
and it prevents issues on their side, because it is not a "whim" of the frontend engineer, it is an actual issue that the DevOps|Backend|Frontend
teams need to care about.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
If we have actual documentation on the why, then an engineer can go into their superiors and say "this pattern we are using is wrong because it can cause issues in downstream libraries, here is the docs".
I totally agree with you 👌
My point is that if this message can be understood as "you should not do it because it's bad" and not "you sould not do it because otherwise our product breaks 😢 " it's better 😆
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Also I'm not blocking this BTW, if we want to have this out ASAP then build on top of it is good for me 👍
Make sure to set `process.env.NODE_ENV` to `'production'` in your build process to avoid the watermark in production. | ||
Most bundlers set this environment variable automatically when building for production, but for custom setups, you might need to set it manually. | ||
|
||
Note that `NODE_ENV=production` is not MUI-specific and is a common practice in the JavaScript ecosystem. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We almost never use MUI standalone in the docs. Like the React docs almost never mention Meta. A quick patch: 06e1e72.
Preview: https://deploy-preview-13869--material-ui-x.netlify.app/x/introduction/licensing/#3-expired-license-key