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

Support .js config files in "type": "module" projects? #29

Closed
michael42 opened this issue Mar 22, 2022 · 6 comments
Closed

Support .js config files in "type": "module" projects? #29

michael42 opened this issue Mar 22, 2022 · 6 comments

Comments

@michael42
Copy link
Contributor

Hi there,

would you consider supporting .js files inside projects with "type": "module" set? Otherwise is pretty hard to provide config files sometimes, because not every user of lilconfig actually includes .cjs as an allowed file extension.

Currently I'm using the following patch, which should work for CommonJS and ECMAScript Modules since Node 12, so it would require a semver-major bump (or a fallback for Node 10):

diff --git a/dist/index.js b/dist/index.js
index a153607e305eb6d40b735aa176eee388b8172dc9..497efc7dc60165b306f46211fb7ddd811b51ef72 100644
--- a/dist/index.js
+++ b/dist/index.js
@@ -28,7 +28,7 @@ function getSearchPaths(startDir, stopDir) {
     }, { searchPlaces: [], passedStopDir: false }).searchPlaces;
 }
 exports.defaultLoaders = Object.freeze({
-    '.js': require,
+    '.js': id => import(id).then(mod => mod.default),
     '.json': require,
     '.cjs': require,
     noExt(_, content) {
@@ -123,7 +123,7 @@ function lilconfig(name, options) {
                 }
                 else {
                     validateLoader(loader, loaderKey);
-                    result.config = loader(filepath, content);
+                    result.config = await loader(filepath, content);
                 }
                 result.filepath = filepath;
                 break;
@antonk52
Copy link
Owner

Hi and thank you for the issue.

Since this package attempts to mimic cosmiconfig, I would rather to wait for them to make a major release with support for ESM native node modules and drop nodejs v10. From my understanding it is not yet implemented as nodejs does not offer an API to clear cache for imported modules like require.cache which is a blocker for cosmiconfig.

From your diff I can see that there is currently an issue with lilconfig as loaders can be async for lilconfig function. You can submit a PR to add the two missing awaits and I am happy to merge it and publish a new version that would allow you to provide an ESM loader instead of patching a dependency, ie

const loadESMDefault = p => import(p).then(x => x.default)

lilconfig('myapp', {
  loaders: {
    '.js': loadESMDefault,
    '.mjs': loadESMDefault,
  }
}).load('./path/to/esm/file.js') // Promise<config>

Await is missing here and here.

Let me know what you think about this.

michael42 added a commit to michael42/lilconfig that referenced this issue Mar 23, 2022
@michael42
Copy link
Contributor Author

Thanks for the fast response and great idea splitting the patch. I created a pull request adding the awaits (and learned a bit about the API while writing a test for it).

antonk52 added a commit that referenced this issue Mar 23, 2022
fix: support async loaders with lilconfig.search (#29)
@antonk52
Copy link
Owner

Nicely done. I've released a new version v2.0.5 with your fix, this should make it easier for people to consume ESM configs.

@marekdedic
Copy link
Contributor

Hi, it seems to me that osmiconfig now supports this looking at the changelog for version 8.2.0. Could we please reopen this? Happy to send a PR if necessary...

@antonk52
Copy link
Owner

antonk52 commented Jan 5, 2024

Hi @marekdedic this sounds reasonable to me. Happy to review and merge your PR. I’ve reopened the issue.

@antonk52
Copy link
Owner

closed by #46

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

Successfully merging a pull request may close this issue.

3 participants