Skip to content
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

Use dotenv-flow instead of dotenv #14

Open
paulintrognon opened this issue Sep 30, 2020 · 3 comments · May be fixed by #15
Open

Use dotenv-flow instead of dotenv #14

paulintrognon opened this issue Sep 30, 2020 · 3 comments · May be fixed by #15

Comments

@paulintrognon
Copy link

Hi! Thank for this awesome plugin :-)

One problem I have though, I am using Next.js, which has a .env and a .env.local file (the .env.local overrides the .env values), and your plugin does not override the .env with .env.local value.

I guess this is because your plugin is based on dotEnv. Can I suggest you use dotenv-flow instead so we can have the full power of .env.* ?

Cheers :)

paulintrognon added a commit to paulintrognon/cypress-dotenv that referenced this issue Sep 30, 2020
@paulintrognon paulintrognon linked a pull request Sep 30, 2020 that will close this issue
@kamikazePT
Copy link

kamikazePT commented Oct 26, 2020

@paulintrognon

I was just going to do the same as you :D
could you release it as a separate package called cypress-dotenv-flow?

This way we can have a dotenv only solution since dotenv-flow is an extension and the reason people usually want dotenv-flow is because they probably either use NextJS or CRA, which replicate the same behaviour or use it under the hood

PS: if you end up making that release let me know, I would like to use it

EDIT: I was in need of this so I've forked your fork and made the necessary changes for it to be a separate package
https://www.npmjs.com/package/cypress-dotenv-flow

the version is the same as the latest of cypress-dotenv, with the purpose to keep future developments in sync (unless a rewrite is done on the one I've just published)

@wujekbogdan
Copy link

@kamikazePT Thanks for the fork.

I have an idea on how to make it more flexible. Instead of having 2 version of the plugin: one for dotenv and one for dotenf-flow I think we could add implementation argument to cypress-dotenv. This argument would accept a reference to dotenv implementation. By default it would be dotenv.

const dotenvPlugin = require('cypress-dotenv');
const dotenvFlow = require('dotenv-flow');

module.exports = (on, config) => {
  return dotenvPlugin(config, {}, false, dotenvFlow)
};

It would be also good to change the parameters syntax. Passing so many parameters isn't very convienient. Especially if normally the second and the third one normally use defaults. So I would refactor the parameters to object syntax:

const dotenvPlugin = require('cypress-dotenv');
const dotenvFlow = require('dotenv-flow');

module.exports = (on, config) => {
  return dotenvPlugin({
    config,
    dotenvOptions: {},
    all: false,
    implementation: dotenvFlow,
  })
};

The downside of this solution is that it breaks backwards compatibility.

@kamikazePT
Copy link

kamikazePT commented Feb 24, 2021

@wujekbogdan
I agree with the object syntax for parameters as well

also great Idea on making the dotenv package an injected dependency! :)

how about

const dotenvPlugin = require('cypress-dotenv');
const dotenvFlow = require('dotenv-flow');

module.exports = (on, config) => {
  return dotenvPlugin({
    config,
    dotenv: [dotenvFlow, { /* dotenv options */ },
    all: false,
  })
};

or if we dont need to pass dotenv options we could do this

const dotenvPlugin = require('cypress-dotenv');
const dotenvFlow = require('dotenv-flow');

module.exports = (on, config) => {
  return dotenvPlugin({
    config,
    dotenv: dotenvFlow,
    all: false,
  })
};

the array/object prop syntax is inspired from config files like when you configure babel plugins/presets... what do you think about that?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants