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
feat(): prioritize custom config value over environment's value when 'ignoreEnvVars' is 'true' #873
Conversation
…'ignoreEnvVars' is 'true' Closes nestjs#245 BREAKING CHANGE: changes the priority of variables loaded from different sources.
I don't want to jump in on this late, but yes, I was googling today to see if this was possible. The current default seems a little illogical, with runtime envvars taking precedent over explicitly set and closer-to-code .env files. Seems like the proximity should call for precedent. I will admit to not knowing much about this topic, as I don't usually have to mix and match runtime envvars with .env files. All this said, I feel like "ignoreEnvVars" is not what this feature is. The env vars will still be read in, they just won't override the .env file's values. Correct? Wouldn't this be clearer if it was |
You’re right. I will fix this and make sure that when this option is enabled, env vars will be totally ignored. Just like the other “ignoreEnvFile” option. |
@mahdavipanah oh thanks for the quick reply! You are going in the opposite direction of my hopes, but I like that you will make My hopes, and what I was googling for, is to mix and match values from both env files and env vars, but to have the env files take precedent over the runtime. I share your goal of "changes the priority of variables loaded from different sources" and I don't personally have the usecase to completely ignore env vars, as you suggest. But I'm not the maintainer, and this is your PR, so feel free to take it whichever direction makes sense for you. |
I completely understand your point of view but this pull request is specifically for this issue: #245. Your point on the naming of ignoreEnvVars made sense and so I reviewed #245 and found out that the discussion on the issue suggested completely disabling the picking of env vars as the name suggests. As for the priority feature you are looking for, I encourage you to open a feature request issue. |
|
I understand your point and agree that it should not have any impact on precedence and therefore need to update this pull request. But isn’t it reasonable to couple this option with the behavior that makes |
We should just introduce a new option for this and I'd be more than happy to accept a PR that does that! |
Closes #245
BREAKING CHANGE: changes the priority of variables loaded from different sources.
PR Checklist
Please check if your PR fulfills the following requirements:
PR Type
What kind of change does this PR introduce?
What is the current behavior?
Issue Number: #245
What is the new behavior?
Does this PR introduce a breaking change?
Other information