Calling setModuleDefaults on empty module causes an error

[gibsonjoshua55]

Note: for support questions, please use StackOverflow and tag your question with node-config. This repository's issues are reserved for feature requests and bug reports.

Before submitting a bug report, please search the issue tracker the wiki first. Many issues have already been discussed.

The wiki is located at: https://github.com/lorenwest/node-config/wiki

I'm submitting a ...

• bug report
• [ ] feature request
• [ ] support request or question => Please do not submit support request or questions here, see note at the top of this template.

What is the current behavior?

If the current behavior is a bug, please provide the steps to reproduce and if possible a minimal demo of the problem:

If you have a config that does not include in a module, calling setModuleDefaults for that module causes an error.

For example if my config resolves to the following object:

{
    "module1": {
        "setting": "value"
    }
}

and I call config.util.setModuleDefault('module2', {a: "value"}), I will get a cannot read property 'module2' of undefined error.

What is the expected behavior?

I would expect that if the module does not exist, the module should be added to the config.

Please tell us about your environment:

• node-config version: 3.2.2
• node-version: 12.8.1

Other information

The problem appears to be related to getImpl returning undefined for the module, which might be the right behavior for that function, but then that undefined value is passed into extendDeep. Perhaps if the result of getImpl is undefined or null, an empty object should be passed into extendDeep.

[markstos]

The current behavior helps catch typos when you typed module2 but meant module1.

As alternate resolution, what about creating module2 and setting it to an empty object? That would clearly express your intent.

Changing the behavior may address your desire but cause silent failure for someone else who expected node-config to throw an exception if an expected module was not found.

[gibsonjoshua55]

I can understand that. In some file formats, that's just not always the clearest. For example in yaml,

module2:

still resolves to undefined. You can do

module2: {}

but then you start mixing json and yaml which feels a little weird and might be prone to be deleted or missed by a dev who would be somewhat confused why that is there. It'd be nice to not have to include that in every app that will use my library to set it's configs, but maybe that's necessary.

[wong2]

this bothers me either, now I have to tell everyone using my library to add an empty config in their project, instead of relying on the default behavior

[lorenwest]

I agree @wong2 - sub module support could use an overhaul - see #572

If you see a temporary solution to this issue until the overhaul happens, feel free to issue a PR

[jdmarshall]

Does this still happen? I am working on some new code that will use setModuleDefaults() and I feel like if this were still an open issue I should have tripped over it massively already.