indexzero / nconf

Hierarchical node.js configuration with files, environment variables, command-line arguments, and atomic object merging.
https://github.com/indexzero/nconf
MIT License
3.87k stars 255 forks source link

esbuild cannot bundle nconf #386

Closed xfournet closed 1 year ago

xfournet commented 3 years ago

Due to dynamic require of stores, esbuild cannot bundle nconf. See following warning when bundling with esbuild:

 > ../node_modules/nconf/lib/nconf.js:28:15: debug: This call to "require" will not be bundled because the argument is not a string literal
    28 │         return require('./nconf/stores/' + store)[name];

Since the list of builtin stores is static, they're is no need to make dynamic require.

Possible workaround: specify an esbuild inject scripts, eg esbuild --bundle --inject:esbuild-inject.js src/index.js with esbuild-inject.js

import nconf from 'nconf';
import { Argv } from 'nconf/lib/nconf/stores/argv';
import { Env } from 'nconf/lib/nconf/stores/env';
import { File } from 'nconf/lib/nconf/stores/file';
import { Literal} from 'nconf/lib/nconf/stores/literal';
import { Memory} from 'nconf/lib/nconf/stores/memory';

// avoid dynamic require() in nconf
Object.defineProperty(nconf, 'Argv', { get: () => Argv });
Object.defineProperty(nconf, 'Env', { get: () => Env });
Object.defineProperty(nconf, 'File', { get: () => File });
Object.defineProperty(nconf, 'Literal', { get: () => Literal});
Object.defineProperty(nconf, 'Memory', { get: () => Memory});
export { nconf };

However it would be more convenient to avoid that.

adenhertog commented 2 years ago

I'm also getting hit by this, can a publish be done with the fix commit - https://github.com/indexzero/nconf/commit/ce212b2f1dbf96cee001b5f621979c564638f0e7 ? 🙏

kahluagenie commented 1 year ago

would love to see a publish of this as well!

mhamann commented 1 year ago

yes, this would be good to backport and publish. my only hesitation is that defineGetter is deprecated. We can probably replace it with Object.defineProperty() instead.

Jamie-BitFlight commented 1 year ago

If anyone still has this issue, you can work around it using this fix. https://gist.github.com/Jamie-BitFlight/d15ad615249d60b02dfb72e0162ad5f9

xfournet commented 1 year ago

@mhamann defineGetter is what is currently used in nconf 0.12.0 and the patch don't change anything regarding that. You could probably review what is better in another issue. I created a backport PR in v0.x branch (#402). Would be possible to release a new 0.x version with the fix ?

mhamann commented 1 year ago

nconf v0.12.1 has been released with this change.