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

Error when using require to load an ES6 module #59

Open
mackie1311 opened this issue Aug 30, 2021 · 7 comments
Open

Error when using require to load an ES6 module #59

mackie1311 opened this issue Aug 30, 2021 · 7 comments

Comments

@mackie1311
Copy link

mackie1311 commented Aug 30, 2021

Hi,

First off great NPM package by the way as well as your DI solution truly excellent work. Second, When I try to use the manager.require() method with my ES6 module I get the following error:
Cannot use import statement outside a module
If I install the package directly and use it with an import statement it works. I have check my package JSON sets the type to "module" which it does but I still get the error.
Is this intentional that import is not supported or is this a bug. Or am I doing something wrong here? Sample code block of my import attempt below

async installCustomPluginLibrary (customPackage, version) {
    await this._manager.installFromNpm(customPackage, version)
    try {
      const newPluginLib = this._manager.require(customPackage)
      const plugins = newPluginLib.libraryPlugins
      const installed = []
      for (let i = 0; i < plugins.length; i++) {
        const plugin = {
          parentPackage: customPackage,
          parentPackageVersion: version,
          key: plugins[i].pluginKey(),
          name: plugins[i].pluginName(),
          description: plugins[i].pluginDescription(),
          params: plugins[i].parameters
        }
        console.log('starting creation of plugin ' + plugins[i].pluginKey())
        console.log(plugin)
        moduleService.CreateModulePlugin(plugin)
        console.log('Plugin ' + plugins[i].pluginKey() + ' created in plugin store')

        installed.push(plugin)
      }
      return installed
    } catch (error) {
      console.log(error)
      throw error
    }
  }

The install works but the line const newPluginLib = this._manager.require(customPackage) fails with the error above. I am going to try changing my files from .js to .mjs to see if that works and will report back. I would appreciate some info though as to whether this is supposed to work or not.

Update:
I tried changing my ES6 module file types to .mjs and the live-plugin-manager failed with the following error: Invalid javascript file

Thanks!,

@davideicardi
Copy link
Owner

Hi @mackie1311
I suspect that for now it is not possible to import ES6 module.
Probably using https://nodejs.org/api/vm.html#vm_class_vm_module we should be able to implement it, but nerver yet tried.

As usual any help is appreciated, otherwise I will try to work on that in the future.

@davideicardi davideicardi changed the title Error when using require to fetch a plugin Error when using require to load an ES6 module Aug 30, 2021
@mackie1311
Copy link
Author

Thanks, @davideicardi I will set aside some time to look into this as well but for now, using CommonJS format and everything works great! I Will keep you posted

@b3nab
Copy link

b3nab commented Oct 23, 2021

Ciao @davideicardi, I quote all words from @mackie1311 you have done really a good job.

I'm planning to use your package in some way to orchestate an extension manager for b3nab/deckpad. Probably you need to be aware that ESM now is getting traction and can be used without experimental flags at minimum Node.js 12.20, 14.14, or 16.0.
To understand more about the "Pure ESM" you can take a look at this discussion https://gist.github.com/sindresorhus/a39789f98801d908bbc7ff3ecc99d99c.

If you want I could contribute to this package with a PR if I will find out a way. 😄

To help continue the discussion reagarding an improvement on live-plugin-manager interoperability with cjs and esm I will leave some interesting articles and links:
https://blog.logrocket.com/how-to-use-ecmascript-modules-with-node-js/
https://redfin.engineering/node-modules-at-war-why-commonjs-and-es-modules-cant-get-along-9617135eeca1
https://nodejs.org/api/esm.html
https://nodejs.org/api/esm.html#interoperability-with-commonjs

@davideicardi
Copy link
Owner

Thank you for the info @b3nab . I will try to investigate on that. Pull requests are always welcome!

@davideicardi
Copy link
Owner

I have done some investigation, and I can confirm that it should be possible to support ECMAScript Modules.
We should essentially use the vm.Module class. See doc here.
But consider that as of today it is still marked as Experimental.

I think that this will be a major release with no backward compatibility (or at least it can be difficult to be backward compatible...).

Here the steps that I think we should do to implement it:

  1. Implement live-plugin-manager as a module (this is required to allow to use live-plugin-manager as a module, but not stricly related to the ability to load modules)
  2. Review PluginVm class to use the new vm.Module class (this is where we need to change most of the actual code). Essentially instead of using a vm.Script we should use a vm.Module ... with all the required changes.
  3. replace the current PluginManager.require with a PluginManager.importModule function that return a Promise (marked as async) and call the new PluginVm functions.

@christianhugoch
Copy link

Hi, quoting @mackie1311, too :). Was there any progress on this?

@davideicardi
Copy link
Owner

@christianhugoch Sorry, no news from me. I wasn't able to work on it. Any help is appreciated.

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

No branches or pull requests

4 participants