Support for node ES6 modules

[richardsimko]

Since Node supports ES6 modules natively in 14.x and up I tried using it with Awilix, however since Awilix uses require() it fails with the following error:

Error [ERR_REQUIRE_ESM] [ERR_REQUIRE_ESM]: Must use import to load ES Module:

I tried some experimenting with dynamic import using import() however since this returns a Promise I think the library needs to be rewritten quite a bit in order to support this.

Is this something you have planned going forward? I'm happy to help out but I would probably need some pointers as to where to start.

[jeffijoe]

I would suggest not using loadModules then. I also don’t personally use ES modules natively on the server, I see no reason to yet.

[richardsimko]

Should I take that as a "No, this will not be supported"? Not using loadModules isn't really an option since that would involve having to import each file manually and that takes away a big chunk of what makes awilix great.

When maintaining a library used by thousands of teams around the world then, no offence, but what you personally use or don't use is of little relevance. ES6 modules will be used more frequently going forward and I won't be the last person to encounter this.

Again, I'm happy to help out and submit a PR if you have any ideas on what needs to be changed in order for loadModules to support async loading of modules.

[jeffijoe]

Having to make loadModules async and use dynamic import is a breaking change. Whats the lowest version of Node that supports this ootb? I don’t want to cut off older versions just yet. Does it work even when the loaded module is CJS?

There is nothing stopping you from writing a load modules impl that uses listModules and dynamic import externally.

[jeffijoe]

What I personally do and don’t do is relevant because often times people are doing things with Awilix that they shouldn’t. That might not be the case here but me stating that I don’t use ES modules on the server because I don’t see any additional value yet isn’t hurting anybody.

[richardsimko]

Having to make loadModules async and use dynamic import is a breaking change. Whats the lowest version of Node that supports this ootb? I don’t want to cut off older versions just yet. Does it work even when the loaded module is CJS?

Yeah, that's the drawback, another option would be to add it as an option or create a separate function. None of the solutions are great but I think enforcing the async behavior is not a good idea, especially since I imagine a lot of calls to loadModule are done from the root level and async/await in root level code is also only supported on Node 14.x+.

Node 14 is the first to support this natively, so it's still fairly new. However simply because loadModules is made async doesn't mean that support for older Node versions has to be dropped. Once it's made async it should be very easy to switch between using require and import, it's just a matter of changing this line

awilix/src/load-modules.ts

Line 99 in 10ab656

const loaded = dependencies.require(m.path)

(Unless of course there are other side effects which I haven't considered)

Doing import() of a CJS module works, however as I mentioned I think import() is only available on Node 14.0+ so it's not a good idea to use this exclusively.

What I personally do and don’t do is relevant because often times people are doing things with Awilix that they shouldn’t. That might not be the case here but me stating that I don’t use ES modules on the server because I don’t see any additional value yet isn’t hurting anybody.

That's fair, I interpreted your original comment as "I'm not using this so it's not relevant". My bad :)

[jeffijoe]

I guess adding a loadModulesAsync that uses import would work, but then later when we remove require completely it will be weird, because we'd want to just call it loadModules again.

Not sure what the best approach is. I was holding off on it because ES Modules is so new that I am not comfortable supporting it yet. Since I don't use it myself it will be difficult for me to troubleshoot if there are issues with it.

[richardsimko]

We're quite heavily invested into both ES6 modules (Currently using standard-things/esm but we want to drop that in favor of native) and Awilix so we'd be happy to guinea pig the setup.

Would it perhaps be possible to add it as an option to loadModules? Something like {useEs6Import:true} (With default false of course) which also makes it async and take care of the split internally? I'm not sure if TS supports conditional return types based on the input but if it can return a Promise or what it currently returns based on the option that would probably be the most forwards and backwards compatible solution.

[jeffijoe]

TS does support conditional return types. Here is what you can do:

Add an esModules option to the options object. The generic will be used for the conditional type later.

export interface LoadModulesOptions<ESM extends boolean = false> {
  // ...
  esModules?: ESM
}

Change the loadModules signature to conditionally return a promise based on the value of that option.

export interface LoadModulesResult {
  loadedModules: Array<ModuleDescriptor>
}

export function loadModules<ESM extends boolean = false>(
  dependencies: LoadModulesDeps,
  globPatterns: string | Array<string | GlobWithOptions>,
  opts?: LoadModulesOptions<ESM>
): ESM extends true ? Promise<LoadModulesResult> : LoadModulesResult

When called with false or no option at all:

When called with true:

[richardsimko]

Great, thanks for the input! I'll give it a try tomorrow and see what I can do!

[richardsimko]

I created #211 with a fix for this!