Fix for collision of setModuleDefaults with get(), has() and friends

[jdmarshall]

This PR addresses #689

The problem is that defineProperty() defaults to configurable: false, which should only be false in makeImmutable(). If you set it before that, you can't delete properties or modify the property.

[jdmarshall]

This PR may also address a couple of the bullet points in #226

[jdmarshall]

Anything I can do to help with getting this PR approved?

I’m trying to remove a bespoke implementation of module defaults that was written for a project I work on. It’s mutually recursive (!), slow and memory hungry. This is the last bug I need fixed in node-config to remove that code in favor of setModuleDefaults()

happy to explain or add more code coverage

[KernelDeimos]

Giving this a +1, looks sane to me

[markstos]

@jdmarshall In the related issue, you said you had a problem that was sometimes triggered and sometimes not. Why was the issue sometimes triggered and sometimes not? From looking at the patch here, either configurable is true or false. Ithe current configurable behavior here was the bug, wouldn't it fail consistently, not just sometimes?

[jdmarshall]

We have a very complicated use case, 3 apps that use some of our modules, a peculiar dev situation, an armful of CLIs, and several NodeJS versions. Some would work and some wouldn’t. That’s what I meant.

This PR comes with a test that reproduces the underlying issue.

[jdmarshall]

Any progress on this? I’ve had to rename a bunch of config and add validation to disallow “get” as a key in our config files to prevent this happening to us. So many config files.

[jdmarshall]

I've been using a workaround for this issue for 13 months now. @markstos what needs to be done here? The original code misunderstands how defineProperty works in the absence of explicit settings, resulting in calls that do not accomplish what is desired.

Then later attempts to define module defaults fail silently, which is the behavior when attempting to assign values to a property that is not defined as mutable.