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

esbuild cannot bundle nconf #386

Closed
xfournet opened this issue Oct 6, 2021 · 6 comments
Closed

esbuild cannot bundle nconf #386

xfournet opened this issue Oct 6, 2021 · 6 comments

Comments

@xfournet
Copy link
Contributor

xfournet commented Oct 6, 2021

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.

xfournet added a commit to xfournet/nconf that referenced this issue Oct 6, 2021
xfournet added a commit to xfournet/nconf that referenced this issue Oct 6, 2021
@adenhertog
Copy link

I'm also getting hit by this, can a publish be done with the fix commit - ce212b2 ? 🙏

xfournet added a commit to xfournet/nconf that referenced this issue Apr 13, 2022
@kahluagenie
Copy link

would love to see a publish of this as well!

@mhamann
Copy link
Collaborator

mhamann commented Sep 20, 2023

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
Copy link

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

xfournet added a commit to xfournet/nconf that referenced this issue Oct 22, 2023
@xfournet
Copy link
Contributor Author

@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 pushed a commit that referenced this issue Oct 23, 2023
@mhamann
Copy link
Collaborator

mhamann commented Oct 23, 2023

nconf v0.12.1 has been released with this change.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

5 participants