feat: custom prefix #482

vuode · 2021-12-26T17:27:05Z

Custom prefix

In #480 @niieani suggested a great feature—ability to use custom prefixes.

Here is my attempt to implement this feature

// default
new Dotenv({})
// process.env.PORT

// explicit
new Dotenv({
  prefix: 'process.env.'
})
// process.env.PORT

// custom prefix
new Dotenv({
  prefix: `import.meta.env.`
})
// import.meta.env.PORT

niieani

Thanks so much for implementing this! This looks great, I've left a few notes though.
🙇

niieani · 2021-12-27T08:01:51Z

src/index.js

@@ -150,8 +152,9 @@ class Dotenv {
 // However, if someone targets Node or Electron `process.env` still exists, and should therefore be kept
 // https://webpack.js.org/configuration/target
 if (this.shouldStub({ target, version })) {
+ const replace = this.config.prefix.slice(0, -1) // Remove the dot in the end of prefix


I'm not sure about this assumption here. Basically it means that the dot has to be at the end of the prefix. But if it has to be there, why not just make it implicit, and remove it from the config option altogether?
i.e. prefix: 'process.env' instead of prefix: 'process.env.'

Thank you for your time for this review! Initially, I tried to account for the case, suggested by @mrsteele:

new Dotenv({ prefix: `MY_TEST_` }) // MY_TEST_PORT

But I guess for now it's better to stay with implicit . separator, as it makes config more readable and less error prone for most use cases.

ah, I see. Okay, I'll let @mrsteele make the final call on this one, since it's his package. 👍

Yeah I suggest we be as explicit as we can here. I don’t want to back us into a corner.

make the dot explicit please 🙏

niieani · 2021-12-27T08:04:34Z

src/index.js

@@ -150,8 +152,9 @@ class Dotenv {
 // However, if someone targets Node or Electron `process.env` still exists, and should therefore be kept
 // https://webpack.js.org/configuration/target
 if (this.shouldStub({ target, version })) {


I think shouldStub should be made to return true only when prefix is process.env (based on the reasoning in the comment above)

Thank you! I've made changes, now it stubs only when prefix is process.env

Yeah, the water is getting a little muddy here. Originally the system implied based on env (which I think is safe). Perhaps the stub can work as is now, but if we see the prefix as manually set we force the stub you defined always.

Sorry, I didn't fully understand the intended behaviour when the prefix is manually set. Should we continue to stub process.env or the set prefix?

To keep this as a non-major version update so people can update without things breaking I suggest we let the system work as it used to, but should they explicitly set the “prefix” property it would always apply the stub with whatever they have requested.

Am I getting it right?

// Current behaviour new Dotenv() // process.env.MISSING --> "MISSING_ENV_VAR".MISSING new Dotenv({ ignoreStub: true }) // process.env.MISSING --> process.env.MISSING

// With set option new Dotenv({ prefix: 'META_ENV_' }) // META_ENV_MISSING --> "MISSING_ENV_VAR".MISSING // process.env.MISSING --> process.env.MISSING new Dotenv({ prefix: 'META_ENV_', ignoreStub: true }) // META_ENV_MISSING --> META_ENV_MISSING // process.env.MISSING --> process.env.MISSING

No. We shouldn’t be changing anything about how it works currently and you have changed how the plug-in works out-of-the box (if I am reading this right).

Reading some of these assumptions I’m also worried the original point of the stub has been lost.

the basic requirements of this pr is quite simple:

Should support a “prefix” configuration property and should be able to override the existing process.env. prefix (which is the default).

Stubbing (which was the last major feature introduced to account for all the unique webpack targets) should work as it does today, but if you override the default prefix you should also force the stub (using of course the defined prefix in the configuration).

With that being said, I do think it’s work getting rid of the stub feature and allowing users to just manually set to an empty string to remove the stub for unique targets, but it’s a little more debatable as that also introduces other complexities.

The plug-in stubs process.env with "MISSING_ENV_VAR" out-of-the-box when the env var is missing, I didn't change that, plug-in works as it did before, unless a prefix property is set. I've illustrated this with the code example with comment "Current behaviour".

The second example is my understanding of how the plugin should work when the property is set.

I've made the dot explicit in the most recent commit, but I didn't change the bit, which disables stubbing when the prefix is set to other value, than process.env.. Currently DefinePlugin can replace full expression or a part of expression, with a dot following it. For us it means that we can't replace every possible user defined prefix, but only those, ending with a dot. That is why I kept this restriction for now.

I can try to explore other possibilities for stubbing custom prefixes, but as I see now, it most likely will involve changes in DefinePlugin.

mrsteele

Hey, happy holidays. Glad to see this thing getting some traction!

left a few comments with my thoughts. The biggest one being that I think we should remove the assumptions about dot notations. I have seen some other systems use underscores for instance, and I would love to support those ideas. (NEXT_PUBLIC_HELLO would have a prefix NEXT_PUBLIC_ for the env variable HELLO).

Let me know if they don’t make sense or if you see some issues or alternatives.

mrsteele · 2021-12-27T18:36:32Z

README.md

@@ -115,6 +117,7 @@ Use the following properties to configure your instance.
 * **expand** (`false`) - Allows your variables to be "expanded" for reusability within your `.env` file.
 * **defaults** (`false`) - Adds support for `dotenv-defaults`. If set to `true`, uses `./.env.defaults`. If a string, uses that location for a defaults file. Read more at [npm](https://www.npmjs.com/package/dotenv-defaults).
 * **ignoreStub** (`false`) - Override the automatic check whether to stub `process.env`. [Read more here](#user-content-processenv-stubbing--replacing).
+* **prefix** (`'process.env'`) - The prefix to use before the name of your env variables.


I think I would just include the dot notation here at the end (we pack DefinePlugin is just looking for raw strings, it also keeps opportunities for other notations as a viable option).

“process.env.” as the prefix.

mrsteele · 2021-12-27T18:37:11Z

README.md

@@ -128,7 +131,8 @@ module.exports = {
 allowEmptyValues: true, // allow empty variables (e.g. `FOO=`) (treat it as empty string, rather than missing)
 systemvars: true, // load all the predefined 'process.env' variables which will trump anything local per dotenv specs.
 silent: true, // hide any errors
- defaults: false // load '.env.defaults' as the default values if empty.
+ defaults: false, // load '.env.defaults' as the default values if empty.
+ prefix: 'import.meta.env' // reference your env variables as 'import.meta.env.ENV_VAR'.


All of these examples here are the defaults. I would adjust yours to show “process.env.” to maintain consistency.

Hello! As I see, the defaults are set only for path, all other options are undefined and coerced to false in conditions. That's why I assumed putting here possible option instead of default. Am I missing something here?

dotenv-webpack/src/index.js

Lines 33 to 35 in bf50d8d

this.config = Object.assign({}, {

path: './.env'

}, config)

Oh wow, you are right I’m not sure where my head was at. You can disregard this bit.

mrsteele · 2021-12-27T18:38:10Z

src/index.js

 * @returns {webpack.DefinePlugin}
 */
 constructor (config = {}) {
 this.config = Object.assign({}, {
- path: './.env'
+ path: './.env',
+ prefix: 'process.env'


Add the trailing dot

mrsteele · 2021-12-27T18:38:34Z

src/index.js

 const formatted = Object.keys(variables).reduce((obj, key) => {
 const v = variables[key]
- const vKey = `process.env.${key}`
+ const vKey = `${prefix}.${key}`


Remove the assumed dot here to allow the prefix system to handle that.

mrsteele · 2021-12-27T18:39:52Z

src/index.js

@@ -150,8 +152,9 @@ class Dotenv {
 // However, if someone targets Node or Electron `process.env` still exists, and should therefore be kept
 // https://webpack.js.org/configuration/target
 if (this.shouldStub({ target, version })) {


Yeah, the water is getting a little muddy here. Originally the system implied based on env (which I think is safe). Perhaps the stub can work as is now, but if we see the prefix as manually set we force the stub you defined always.

mrsteele · 2021-12-27T18:40:23Z

src/index.js

@@ -166,6 +169,8 @@ class Dotenv {

 return targets.every(
 target =>
+ // If configured prefix is 'process.env'
+ this.config.prefix === 'process.env' &&


With my comment above we can trash this bit.

mrsteele · 2022-01-16T02:33:34Z

I haven’t forgotten about this (promise, just went on a vacation with the wife and so technology was mostly a no-no).

mom wrestling with this a bit because I’m wondering if we should rethink the last breaking change for this approach. Should we assume your prefix is “process.env.”? Do we let you set it to nothing if you want nothing? Do we try to make them compatible with one another?

I’m worried about the complexities with juggling the former system which makes some assumptions and adds a bit of confusion with this proposed system which is meant to make things a little more explicit. I like things being as easy as possible to get started, but am also a big fan of giving power to the developer to configure what they need when they need it.

Thoughts or ideas or opinions would be appreciated. Also, appreciate the patience (and my wife does too).

niieani · 2022-01-26T20:08:16Z

I'm fine with this as is. Perhaps being explicit has the additional benefit in that process is usually not a global in the browser environment, so the assumption that it's the default is a bit odd, and only due to it being the legacy. In that respect, using import.meta.env. is a bit more sound, because import.meta is actually a standard that browsers with support for modules have to abide by. Changing the default would be a breaking change though (which I'm fine with). Honestly, I think the final decision here is not that important, as long as it's documented.

mrsteele · 2022-01-27T14:51:27Z

Yeah, good point. I'm going to push this out today. Hopefully all systems work 🤞 .

Thank you so much for your patience through all of this @niieani and @vuode. I hope the community enjoys this added feature 🎉

mrsteele · 2022-01-27T14:52:16Z

@vuode did you mean to close this? It didn't get merged.

vuode · 2022-01-27T14:54:21Z

@vuode did you mean to close this? It didn't get merged.

Sorry, no, wrongly clicked the button

github-actions · 2022-01-27T15:01:16Z

🎉 This PR is included in version 7.1.0 🎉

The release is available on:

Your semantic-release bot 📦🚀

vuode added 3 commits December 26, 2021 20:11

feat: adding custom prefix support

e96bdce

docs: add description for prefix usage

dff09ca

docs: fix typo

52ba2c6

niieani suggested changes Dec 27, 2021

View reviewed changes

feat: disable stubbing when prefix is set

17a55ae

mrsteele requested changes Dec 27, 2021

View reviewed changes

fix: return explicit dot to config

0f55644

niieani approved these changes Jan 26, 2022

View reviewed changes

mrsteele approved these changes Jan 27, 2022

View reviewed changes

vuode closed this Jan 27, 2022

mrsteele reopened this Jan 27, 2022

mrsteele merged commit 96b42f4 into mrsteele:master Jan 27, 2022

github-actions bot added the released label Jan 27, 2022

This was referenced May 6, 2022

Support import.meta.env as an alternative to process.env #480

Closed

Restrict to prefix like CRA does with REACT_APP_x #473

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

feat: custom prefix #482

feat: custom prefix #482

vuode commented Dec 26, 2021 •

edited

Loading

niieani left a comment

niieani Dec 27, 2021

vuode Dec 27, 2021

niieani Dec 27, 2021

mrsteele Dec 28, 2021

niieani Dec 27, 2021

vuode Dec 27, 2021

mrsteele Dec 27, 2021

vuode Dec 27, 2021

mrsteele Dec 28, 2021

vuode Dec 28, 2021

mrsteele Dec 28, 2021

vuode Dec 29, 2021

mrsteele left a comment

mrsteele Dec 27, 2021

mrsteele Dec 27, 2021

vuode Dec 27, 2021

mrsteele Dec 28, 2021

mrsteele Dec 27, 2021

mrsteele Dec 27, 2021

mrsteele Dec 27, 2021

mrsteele Dec 27, 2021

mrsteele commented Jan 16, 2022

niieani commented Jan 26, 2022

mrsteele commented Jan 27, 2022

mrsteele commented Jan 27, 2022

vuode commented Jan 27, 2022

github-actions bot commented Jan 27, 2022

feat: custom prefix #482

feat: custom prefix #482

Conversation

vuode commented Dec 26, 2021 • edited Loading

Custom prefix

niieani left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

mrsteele left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

mrsteele commented Jan 16, 2022

niieani commented Jan 26, 2022

mrsteele commented Jan 27, 2022

mrsteele commented Jan 27, 2022

vuode commented Jan 27, 2022

github-actions bot commented Jan 27, 2022

vuode commented Dec 26, 2021 •

edited

Loading