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

Unable to use deeply nested config or env in plugins #1736

Closed
badsyntax opened this issue May 18, 2018 · 16 comments · Fixed by #7960
Closed

Unable to use deeply nested config or env in plugins #1736

badsyntax opened this issue May 18, 2018 · 16 comments · Fixed by #7960
Labels
existing workaround pkg/server This is due to an issue in the packages/server directory type: unexpected behavior User expected result, but got another

Comments

@badsyntax
Copy link

badsyntax commented May 18, 2018

Current behavior:

When returning deeply nested config in the plugin, an error is shown on startup. "Unable to set property {propertyname} of undefined."

Desired behavior:

When returning deeply nested config in the plugin, the config should be merged into the main config, and no error is shown.

Steps to reproduce:

Simply place the following in your plugins/index.js file.

module.exports = () => ({
  foo: {
    bar: 'baz',
  },
});

Versions

Cypress version 2.1.0

Other info

I did a little bit of digging and it tries to merge this extra config into "resolved" config but as cfg.resolved.foo doesn't exist, it fails.

@chrisbreiding
Copy link
Contributor

Cypress only supports known keys for configuration. This is true whether they're set in cypress.json or the plugins file. We whitelist known keys, so any unknown ones won't make it through.

That said, we could certainly improve the experience by either showing a useful error when you try to set an unknown key, especially since it could be a typo of a known key. Or we could just fix this issue with nested values and make sure they're ignored properly so it doesn't give a cryptic error.

By the way (mostly for my own future reference), I needed to nest the values more deeply to reproduce the error:

module.exports = () => ({
  foo: {
    bar: {
      baz: 'qux',
    },
  },
})

@chrisbreiding chrisbreiding added type: user experience Improvements needed for UX pkg/server This is due to an issue in the packages/server directory severity: minor labels May 18, 2018
@badsyntax
Copy link
Author

Sorry for the belated response. What confused me was that I attempted to use this automocker plugin as advertised on the plugins page, which uses a non-known configuration key, and then attempted to use multiple config files implemented via plugins where i was struck with this error.

So it seems like you could use config in this way (using non-known keys), as the automocker plugin does, but it doesn't work when setting config with plugins.

Either way, even adding this message Cypress only supports known keys for configuration. to the config docs page would certainly help in clarifying the intended usage of the config system.

@brian-mann
Copy link
Member

I think to do this we just need to add a plugins namespace / configuration key that enables you to add key/values for specific plugins.

@chrisbreiding what do you think?

@chrisbreiding
Copy link
Contributor

@badsyntax I'm not sure exactly how the automocker plugin works, but if unknown keys are working for it, it might be a bug in Cypress letting them through.

@brian-mann I think we may need to see more use-cases. In the case of automocker, I think it would be better if they accepted the configuration via the registerAutoMockCommands function. In the case of plugins in the plugins file, configuration can be passed in the plugins file.

We'd need to put some thought in before having cypress.json be the way for plugins to be configured. Some questions we'd need to answer:

  • What if different plugins use the same key for configuration?
  • Should a plugin register which keys it accepts or do we use the name of the plugin by convention?
  • How do we enforce uniqueness?

@paulfalgout
Copy link
Contributor

So this is a problem with nested env variables as well
If your cypress.json is

{
  "env": {
  }
}

and your plugin is

module.exports = (on, config) => {
  config.env.foo = {
    bar: 'baz'
  };
  console.log('config looks correct here:', config);
  return config;
};

You'll get an error that bar cannot be set on undefined.
However if you set your cypress.json to:

{
  "env": {
     "foo": { }
  }
}

no issues.

@mike-rogers
Copy link

In our app we needed to lean on node-config to deal with per-environment configurations. We ended up doing something like:

module.exports = (on, config) => {
    config.env.CONFIG = JSON.stringify(require('config'));
}

in plugins/index.js and something like:

    const config = JSON.parse(Cypress.env('CONFIG'));

in the tests. This approach seems to work fine for deeply nested structures.

@blasvicco
Copy link

As @paulfalgout said if we do something like this in our plugin js index file:

module.exports = (on, config) => {
  config.env.visit_options = {
      auth: {
        password: 'test',
        username: 'test_something',
      },
  };
  return config;
};

Then, in the cypress window we are getting this error:
screen shot

@codybarr
Copy link

Yeah, I was trying to use separate config files per environment following the Config API docs here: https://docs.cypress.io/api/plugins/configuration-api.html#Promises

Nested values in my Cypress.json work fine, but when following the above pattern I'm also getting the same error:

image

Eg. We're trying to use values like this:

// cypress/config/dev.json
{
    "baseUrl": "https://dev-website.com",
    "env":
    {
        "api": "https://api.dev-website.com",
        "users":
        {
            "admin":
            {
                "username": "admin",
                "password": "adminpassword"
            },
            "super":
            {
                "username": "super",
                "password": "superpassword"
            },
        }
    }
}

@jennifer-shehane jennifer-shehane added the stage: ready for work The issue is reproducible and in scope label Oct 20, 2018
@skimi
Copy link

skimi commented Oct 24, 2018

To note, copied @mike-rogers solution and it works great but the plugin needs to return the config:

module.exports = (on, config) => {
    config.env.CONFIG = JSON.stringify(require('config'));
    return config
}

@Dobby89
Copy link

Dobby89 commented Nov 13, 2018

Is there a plan to allow custom configuration properties?

@mike-rogers' solution does work, but it seems very hacky.

@doronnac
Copy link

This issue, along with #1859 makes it a lot more flexible to start Cypress through [ts-]node and pass env vars to it, rather than doing it "the Cypress way" with plugins.

I feel like I'm missing something. Any reason not to do it?

@rafael-anachoreta
Copy link

rafael-anachoreta commented Jun 6, 2019

I'm facing the same issue described here. This is especially convoluted when trying to deal with multiple element entries.

{
  'targetCountries': ['uk', 'de'] //doesn't work - Cannot set property '0' of undefined
}

Workaround:

{
  'targetCountries': 'uk,de' //then split them somewhere else
}

Would be really nice to have a better solution for this though.

@ghost
Copy link

ghost commented Feb 6, 2020

+1 for removing the white listing. I don't see what it's meant to achieve and the rationale for being so restrictive.

Either way, thanks for the great tool!

@thecodejack
Copy link

thecodejack commented Feb 26, 2020

Hey I am having similar issue which setting config via plugin

Here is usecases when I am getting same error

module.exports = (on, config) => {
  // `on` is used to hook into various events Cypress emits
  // `config` is the resolved Cypress config
  on('before:browser:launch', (arg1, arg2) => {
    return true;
  });
  config.arc_cookies = {
    x: 1,
    y: 2
  };
  return config;
};

I also tried following

module.exports = (on, config) => {
  // `on` is used to hook into various events Cypress emits
  // `config` is the resolved Cypress config
  on('before:browser:launch', (arg1, arg2) => {
    return true;
  });
  const arc_cookies = {
    x: 1,
    y: 2
  };
  return arc_cookies;
};

@mccataldo
Copy link

mccataldo commented Jul 23, 2020

@jennifer-shehane is this a duplicate of #7959 and therefore fixed by #7960?

@jennifer-shehane
Copy link
Member

The code for this is done in #7960 and was released in Cypress 4.11.0.

I checked all of the previous provided examples in this thread and they run without error in 4.11.0.

This comment thread has been locked. If you are still experiencing this issue after upgrading to
Cypress v4.11.0, please open a new issue.

@jennifer-shehane jennifer-shehane removed the stage: ready for work The issue is reproducible and in scope label Jul 24, 2020
@cypress-io cypress-io locked as resolved and limited conversation to collaborators Jul 24, 2020
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
existing workaround pkg/server This is due to an issue in the packages/server directory type: unexpected behavior User expected result, but got another
Projects
None yet
Development

Successfully merging a pull request may close this issue.