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

🐛 Bug: esm-utils (mocha.addFile) should support URL #4993

Open
4 tasks done
ggrossetie opened this issue Jun 27, 2023 · 3 comments
Open
4 tasks done

🐛 Bug: esm-utils (mocha.addFile) should support URL #4993

ggrossetie opened this issue Jun 27, 2023 · 3 comments
Labels
area: node.js command-line-or-Node.js-specific status: accepting prs Mocha can use your help with this one! type: bug a defect, confirmed by a maintainer

Comments

@ggrossetie
Copy link

ggrossetie commented Jun 27, 2023

Prerequisites

  • Checked that your issue hasn't already been filed by cross-referencing issues with the faq label
  • Checked next-gen ES issues and syntax problems by using the same environment and/or transpiler configuration without Mocha to ensure it isn't just a feature that actually isn't supported in the environment in question or a bug in your code.
  • 'Smoke tested' the code to be tested by running it outside the real test suite to get a better sense of whether the problem is in the code under test, your usage of Mocha, or Mocha itself
  • Ensured that there is no discrepancy between the locally and globally installed versions of Mocha. You can find them with: node_modules/.bin/mocha --version(Local) and mocha --version(Global). We recommend that you not install Mocha globally.

Description

According to the official Node.js documentation, ES modules are resolved and cached as URLs. However, esm-utils does not support loading files from a URL.

Reference: https://nodejs.org/docs/latest-v18.x/api/esm.html#urls

Steps to Reproduce

  1. Initialize a new project: npm init -y (make sure to use a recent version of Node >= 18)
  2. Install Mocha 10.2.0: npm i mocha@10.2.0 --save
  3. Add type": "module" in package.json
  4. Create two files:

index.js

import Mocha from 'mocha'
import ospath from 'node:path'

const mocha = new Mocha()
mocha.addFile(`file://${ospath.resolve('./suite.js')}`)
await mocha.loadFilesAsync()

mocha.run((failures) => {
  console.log({failures, stats: runner.stats})
})

suite.js

describe('Suite', () => {
  it('should run', () => {
    // OK
  })
})
  1. node index.js
node /path/to/project/index.js
node:internal/modules/cjs/loader:1026
  const err = new Error(message);
              ^

Error: Cannot find module '/path/to/project/file:/path/to/project/suite.js'
Require stack:
- /path/to/project/node_modules/mocha/lib/nodejs/esm-utils.js
- /path/to/project/node_modules/mocha/lib/mocha.js
    at Function.Module._resolveFilename (node:internal/modules/cjs/loader:1026:15)
    at Function.Module._load (node:internal/modules/cjs/loader:871:27)
    at Module.require (node:internal/modules/cjs/loader:1098:19)
    at require (node:internal/modules/cjs/helpers:108:18)
    at Object.exports.requireOrImport (/path/to/project/node_modules/mocha/lib/nodejs/esm-utils.js:53:16)
    at async Object.exports.loadFilesAsync (/path/to/project/node_modules/mocha/lib/nodejs/esm-utils.js:100:20)
    at async file:///path/to/project/index.js:6:1 {
  code: 'MODULE_NOT_FOUND',
  requireStack: [
    '/path/to/project/node_modules/mocha/lib/nodejs/esm-utils.js',
    '/path/to/project/node_modules/mocha/lib/mocha.js'
  ]
}

Using .mjs extension does fail as well:

node /path/to/project/index.js
node:internal/errors:478
    ErrorCaptureStackTrace(err);
    ^

Error [ERR_MODULE_NOT_FOUND]: Cannot find module '/path/to/project/file:/path/to/project/suite.mjs' imported from /path/to/project/node_modules/mocha/lib/nodejs/esm-utils.js
    at new NodeError (node:internal/errors:387:5)
    at finalizeResolution (node:internal/modules/esm/resolve:330:11)
    at moduleResolve (node:internal/modules/esm/resolve:907:10)
    at defaultResolve (node:internal/modules/esm/resolve:1115:11)
    at nextResolve (node:internal/modules/esm/loader:163:28)
    at ESMLoader.resolve (node:internal/modules/esm/loader:841:30)
    at ESMLoader.getModuleJob (node:internal/modules/esm/loader:424:18)
    at ESMLoader.import (node:internal/modules/esm/loader:525:22)
    at importModuleDynamically (node:internal/modules/cjs/loader:1134:29)
    at importModuleDynamicallyWrapper (node:internal/vm/module:438:21) {
  code: 'ERR_MODULE_NOT_FOUND'
}

Expected behavior:

Mocha should be able to load ES modules from URL (file://).

Actual behavior:

Module is not found :(

Reproduces how often:

100%

Versions

  • The output of mocha --version and node_modules/.bin/mocha --version: 10.2.0
  • The output of node --version: v18.16.1
  • Your operating system
    • name and version: Ubuntu 22.04
    • architecture (32 or 64-bit): 64-bit
  • Your shell (e.g., bash, zsh, PowerShell, cmd): bash

Additional Information

It should also support query parameters and fragments. That's particularly useful when you want to load a test suite more than once:

const mocha = new Mocha()
mocha.addFile(`file://${ospath.resolve('./suite.js')}?time=${Date.now()}`)
await mocha.loadFilesAsync()

Otherwise, the file will be loaded only on the first mocha instance:

const mocha = new Mocha()
mocha.addFile(`file://${ospath.resolve('./suite.js')}`)
await mocha.loadFilesAsync()
// mocha has a test suite

const mocha1 = new Mocha()
mocha1.addFile(`file://${ospath.resolve('./suite.js')}`)
await mocha1.loadFilesAsync()
// mocha1 has no test suite because suite.js was not loaded again!

Please note that mocha.dispose, mocha.unloadFiles has no effect (as mentioned in the JSDoc):

Note: does not clear ESM module files from the cache

Thanks!

@plroebuck
Copy link
Contributor

https://docs.translatehouse.org/projects/localization-guide/en/latest/guide/translation/paths_urls.html

You could do this yourself by translating the necessary methods from Java File & URI classes,
but there's much more to implementing your request than was specifically requested.
Better you implement what you need.

Would suggest Mocha itself should not implement due to its wide usage and possible security issues mentioned above.

@ggrossetie
Copy link
Author

Thanks for your reply!

You could do this yourself by translating the necessary methods from Java File & URI classes

Could you please expand on this workaround?

but there's much more to implementing your request than was specifically requested.
Better you implement what you need.

I'm not suggesting to translate paths to URLs (or vice-versa). I was assuming that Mocha relies on Node.js built-in import no? If that's the case then we can delegate the work to Node if the file path starts with file://.

@JoshuaKGoldberg
Copy link
Member

Yeah, this seems to be a bug in Mocha to me. Adding a couple logs:

  • The string passed to mocha.addFile looks like 'file:///Users/josh/repos/repros/suite.js' on my computer
  • The string passed to require is the buggy one with file:/ in the middle:
    return require(file);

That require(file) call only happens if a previous import failed:

return dealWithExports(await formattedImport(file, esmDecorator));

That call is also trying with a path like /Users/josh/repos/repros/file:/Users/josh/repos/repros/suite.js.

Manually removing the characters before the file:// at the start of exports.requireOrImport fixes things.

Summary: I think the issue is that Mocha isn't respecting protocols such as file://, and is putting junk on the path before the protocol. Accepting PRs to fix Mocha to not do that.

Great issue, thanks for the detailed port!

@JoshuaKGoldberg JoshuaKGoldberg changed the title esm-utils should support URL 🐛 Bug: esm-utils (mocha.addFile) should support URL Jan 18, 2024
@JoshuaKGoldberg JoshuaKGoldberg added type: bug a defect, confirmed by a maintainer status: accepting prs Mocha can use your help with this one! area: node.js command-line-or-Node.js-specific and removed unconfirmed-bug labels Jan 18, 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: accepting prs Mocha can use your help with this one! type: bug a defect, confirmed by a maintainer
Projects
None yet
Development

No branches or pull requests

3 participants