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

🚀 Feature: Expose PluginLoader or handleRequires to have other frameworks provide support for the require option #4961

Open
christian-bromann opened this issue Jan 23, 2023 · 6 comments
Labels
area: node.js command-line-or-Node.js-specific status: in discussion Let's talk about it! type: feature enhancement proposal

Comments

@christian-bromann
Copy link
Contributor

Is your feature request related to a problem or a nice-to-have?? Please describe.
For frameworks such as WebdriverIO that integrate Mocha, it would be great to support the require option.

Describe the solution you'd like
For that to happen, Mocha should export the PluginLoader or have the handleRequires exposed from the package.

Describe alternatives you've considered
None.

Additional context
Issue filed in the WebdriverIO project: webdriverio/webdriverio#9490

@christian-bromann christian-bromann added the type: feature enhancement proposal label Jan 23, 2023
@juergba
Copy link
Contributor

juergba commented Feb 5, 2023

@christian-bromann
Your above mentioned webdriverio issue has already been fixed and closed. So would do you need exactly?

Mocha's --require (not identical to Node's --require) is a public option and supports both sync and async modules. It can be used to pre-load any module or to define some mochaHooks.

@christian-bromann
Copy link
Contributor Author

@juergba thanks for getting back. The PR currently imports handleRequires which is not ideal because the location of it can change with every release.

// @ts-expect-error not exposed from package yet, see https://github.com/mochajs/mocha/issues/4961
import { handleRequires } from 'mocha/lib/cli/run-helpers.js'

I think to make the Mocha integration work it would be great to either export PluginLoader or the handleRequires method. Would that be possible? Happy to file a PR.

@juergba
Copy link
Contributor

juergba commented Feb 19, 2023

@christian-bromann yes, it's possible. I'm unsure which way to go wether PluginLoader or handleRequires.

PluginLoader was intended to eventually expose it later to the public, but the changes for your purpose seem to be far more complex.

handleRequires is the method which lays behind the --require option. So it would be a small step to expose it. Can we expose a method of a private module?

Evtl. you can explain your reasons for choosing the latter way.
Then your PR would be welcome, thank you.

@JoshuaKGoldberg
Copy link
Member

@christian-bromann I think this is still waiting on you: can you explain why you chose handleRequires over PluginLoader? Does PluginLoader not provide enough API surface for you?

We the new maintainers are still ramping up (#5027) and this is a relatively niche use case (API request for large API user), so it might be a while before we have the time to deeply understand this on our own. Any help you can give us & context on why you implemented things the way you did would be very helpful. 🙂 Thanks!

@JoshuaKGoldberg JoshuaKGoldberg added the status: waiting for author waiting on response from OP - more information needed label Dec 27, 2023
@JoshuaKGoldberg JoshuaKGoldberg changed the title Expose PluginLoader or handleRequires to have other frameworks provide support for the require option 🚀 Feature: Expose PluginLoader or handleRequires to have other frameworks provide support for the require option Dec 27, 2023
@christian-bromann
Copy link
Contributor Author

Hey @JoshuaKGoldberg , I could use the PluginLoader but would need to re-implement all the other logic within handleRequires. All I need is a reliable way to copy what the Mocha CLI is doing so I can easily integrate it in WebdriverIO. For now, I am importing the helper function from Mocha directly:

// @ts-expect-error not exposed from package yet, see https://github.com/mochajs/mocha/issues/4961
import { handleRequires } from 'mocha/lib/cli/run-helpers.js'

which works but is not ideal.

@JoshuaKGoldberg JoshuaKGoldberg removed the status: waiting for author waiting on response from OP - more information needed label Dec 28, 2023
@JoshuaKGoldberg
Copy link
Member

Got it, thanks. I took a read through the code and it looks pretty reasonable what wdio is doing: using Mocha's handleRequires to mimic what the mocha --require does on the CLI.

handleRequires is actually a pretty small wrapper around the PluginLoader class. Removing comments and logs from its current implementation roughly leaves:

async function handleRequires(requires = [], { ignoredPlugins = [] } = {}) {
  const pluginLoader = PluginLoader.create({ ignore: ignoredPlugins });

  for await (const mod of requires) {
    let modpath = mod;
    if (fs.existsSync(mod) || fs.existsSync(`${mod}.js`)) {
      modpath = path.resolve(mod);
    }

    const requiredModule = await requireOrImport(modpath);
    if (requiredModule && typeof requiredModule === "object") {
      pluginLoader.load(requiredModule);
    }
  }

  return await pluginLoader.finalize();
}

Per #4961 (comment):

PluginLoader was intended to eventually expose it later to the public

I think it'd make most sense to provide PluginLoader as a public export. That way API integrators such as yourself can work with its logic as they need without being tied to the fs calls and requireOrImport implementation from Mocha.

Note also that #1457 discusses a separate concept of a "Plugin API", which is a kind of unfortunate naming conflict. One of the two will need to be renamed. No thoughts yet on which.

cc @mochajs/maintenance-crew as this would be a big new API piece - thoughts?

@JoshuaKGoldberg JoshuaKGoldberg added area: node.js command-line-or-Node.js-specific status: in discussion Let's talk about it! labels Feb 7, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area: node.js command-line-or-Node.js-specific status: in discussion Let's talk about it! type: feature enhancement proposal
Projects
None yet
Development

No branches or pull requests

3 participants