[BUG] TypeError: Cannot redefine property when sub module is using config.get() within the module

[hershmire]

Describe the bug

I'm trying to use node-config within my company's ecosystem (apps and libraries) but running into an issue when I start using it within one of our libraries and a consumer instantiates the library more than once. Everything works fine if the library references the config via dot notation. However, if it references values of the config via the recommended config.get('...') method I'll get the following error:

/Users/example/config-module-example/node_modules/config/lib/config.js:423
      Object.defineProperty(object, propertyName, {
             ^

TypeError: Cannot redefine property: myModule
    at Function.defineProperty (<anonymous>)
    at Config.util.makeImmutable (/Users/example/config-module-example/node_modules/config/lib/config.js:423:14)
    at Config.get (/Users/example/config-module-example/node_modules/config/lib/config.js:170:12)
    at myModule (/Users/example/config-module-example/src/my-module.js:18:24)
    at Object.<anonymous> (/Users/example/config-module-example/src/index.js:7:1)
    at Module._compile (node:internal/modules/cjs/loader:1105:14)
    at Object.Module._extensions..js (node:internal/modules/cjs/loader:1159:10)
    at Module.load (node:internal/modules/cjs/loader:981:32)
    at Function.Module._load (node:internal/modules/cjs/loader:822:12)
    at Function.executeUserEntryPoint [as runMain] (node:internal/modules/run_main:77:12)

Here is a repo that demonstrates the issue with a contrived example (reference the README for more details): https://github.com/hershmire/config-module-example.

Expected behavior

I would expect that I could use the get() method off the config object based on your documentation. However, this doesn't seem possible since the .get() method will make the object and its children immutable, making it problematic when the second instance is created. I cannot find any documentation walking through a recommended approach other than what is referenced here (which I seem to be following) – https://github.com/node-config/node-config/wiki/Sub-Module-Configuration.

Please tell us about your environment:

• node-config version: 3.3.8
• node-version: v16.16.0

[hershmire]

👋🏼 Any thoughts on the issue above?

[eiskalteschatten]

We've also had that problem since version 3.3.8 came out. My colleague and I have been working on it for two days and the only real way we've found to solve it is to fix the version to 3.3.7.

The problem seems to be this merge: #516. If I go into the config.js in node_modules and revert the change by hand, it works fine.

[hershmire]

@lorenwest @markstos was this intentional? If so, what's the proposed direction when handling modules?

[markstos]

@hershmire The regression was not intentional. I've asked @fgheorghe who wrote the patch #516 to investigate. Ideally we'd like that fix without regressions, but maybe we can't have both and have to choose.

[jdmarshall]

@markstos you sure about that?

If you unroll their code they are running:

config.util.setModuleDefaults('myModule', defaults);
const apple = config.get('myModule.apple');

config.util.setModuleDefaults('myModule', defaults);
const apple = config.get('myModule.apple');

Which I'm pretty sure the docs imply you should not do.

My read on setModuleDefaults is that you should not be calling it when running a function in your module, you should be running it when the module loads. That way repeated calls do not trigger this problem.

[NilSet]

I just ran into this problem when resolving two different versions of a package in my dependency tree. The package tries to call config.util.setModuleDefaults('myModule', defaults); in both versions, at module load.

How can a package ensure it has defaults set in an idempotent way?

I realize that its somewhat dangerous for two different versions of a library to be setting default values, because the defaults might change between versions, causing default values from the past or future of the package to be seen, but in the case where the defaults are the same, maybe it could be made to be a no-op instead of breaking?

[markstos]

@NilSet It seems like each sub-module should have a private configuration. The best way to do this may be with breaking changes. It was addressed in this fork, which should perhaps be published as it's own module.

#569