ES6 module import support

[katjuncker]

I've been using LoopBack4 from a project that is setup as an ES6 module. The issue is that the lb4 bootloader is unable to scan and import my classes because it is using require() and this is not supported in ES6 context.

I have a fork with a fix for this that allows the bootloader to detect the support for ES6 imports and use them, falling back to require() otherwise. I have tested this on node versions from 10 to 16 and it works fine with all unit tests passing.

Before I go ahead and submit a PR, is there anything I should be aware of? I'm a bit concerned because my solution for detection of the feature uses the "new Function()" which relies upon eval(). This feels a bit hacky, but as far as I can tell is the only valid way to perform this kind of feature detection.

Any feedback would be appreciated.

Forked branch with the fix: https://github.com/sheldonjuncker/loopback-next/tree/es6-bootloading
The primary affected code is in /packages/boot/src/booters/booter-utils.ts

/**
 * Returns an Array of Classes from given files. Works by requiring/importing the file,
 * identifying the exports from the file by getting the keys of the file
 * and then testing each exported member to see if it's a class or not.
 *
 * This detects the support of dynamic imports and uses them if available
 * as this allows ES6 modules to be imported.
 *
 * @param files - An array of string of absolute file paths
 * @param projectRootDir - The project root directory
 * @returns An array of Class constructors from a file
 */
export async function loadClassesFromFiles(
  files: string[],
  projectRootDir: string,
): Promise<Constructor<{}>[]> {
  //Creates an importer which can import using either a dynamic import or require
  const importFile = async (file: string) => {
    try {
      //Try to import in a way that supports ES6 modules
      return await Promise.resolve(
        new Function('file', 'return import(file)')(file),
      );
    } catch (e) {
      //Fallback to require if dynamic import is not supported
      return require(file);
    }
  };

  const classes: Constructor<{}>[] = [];
  for (const file of files) {
    debug('Loading artifact file %j', path.relative(projectRootDir, file));
    const moduleObj = await importFile(file);
    for (const k in moduleObj) {
      const exported = moduleObj[k];
      if (isClass(exported)) {
        debug('  add %s (class %s)', k, exported.name);
        classes.push(exported);
      } else {
        debug('  skip non-class %s', k);
      }
    }
  }

  return classes;
}

[achrinza]

This is something we are open to implementing in LoopBack 4's booter.

I'm a bit concerned because my solution for detection of the feature uses the "new Function()" which relies upon eval().

This is a concern; Is there any reason why we can't just call import() directly?

[katjuncker]

The reason to use the Function approach was to support older versions of node for which this is a syntax error. Even in Node 12 the dynamic import is only available with the --experimental-modules flag.

[achrinza]

The good news is that we've since dropped Node.js v12 support due to its EOL: 5f66e5b

From my understanding, this means we can use import() directly without needing to fallback to require().

[katjuncker]

Oh, that makes this very simple. Do you want me to update my fork and submit a PR?

[achrinza]

That'd be great! Do add a test-case to cover this enhancement.

[ecurien]

Hi,
I've created PR #8754 before discovering this discussion.
I've no Idea why with this one of the typeorm integration test does not path.
Did you have the problem as well ?
Any clue ?
Thank you