jasmine / jasmine-npm

A jasmine runner for node projects.
MIT License
376 stars 145 forks source link

Jasmine 4 ES Modules / Dynamic Import Doesn't Work with Paths that Have Encoded Slashes #196

Closed chucknelson closed 1 year ago

chucknelson commented 2 years ago

Versions

@types/jasmine@4.0.3
jasmine-core@4.1.1
jasmine@4.1.0

Issue

Jasmine 4 moving to dynamic import seems to break tests that run within a path that has an encoded slash.

Per https://jasmine.github.io/setup/nodejs.html#using-es-modules:

Jasmine loads your code using dynamic import, which should be compatible with both ES modules and CommonJS modules. ... But if necessary you can configure Jasmine to load scripts using require by adding "jsLoader": "require" to your Jasmine config file. If you have code that works with "jsLoader": "require" but not without it, please let us know.

If I have a spec in the following path, it works fine:

my-project/src/my-test.spec.js

If the path has an encoded slash, though, it fails with the following error:

my%2Fproject/src/my-test.spec.js

TypeError [ERR_INVALID_MODULE_SPECIFIER]: Invalid module "/my%2Fproject/src/my-test.spec.js" must not include encoded "/" or "\" characters imported from /my%2Fproject/node_modules/jasmine/lib/loader.js
    at new NodeError (internal/errors.js:322:7)
    at finalizeResolution (internal/modules/esm/resolve.js:283:11)
    at moduleResolve (internal/modules/esm/resolve.js:731:10)
    at Loader.defaultResolve [as _resolve] (internal/modules/esm/resolve.js:842:11)
    at Loader.resolve (internal/modules/esm/loader.js:89:40)
    at Loader.getModuleJob (internal/modules/esm/loader.js:242:28)
    at Loader.import (internal/modules/esm/loader.js:177:28)
    at importModuleDynamically (internal/modules/cjs/loader.js:1028:27)
    at exports.importModuleDynamicallyCallback (internal/process/esm_loader.js:30:14)
    at Loader.importShim [as import_] (/my%2Fproject/node_modules/jasmine/lib/loader.js:56:3) {
  code: 'ERR_INVALID_MODULE_SPECIFIER'
}

This same error will happen if the spec is the only thing in an encoded path, like:

/my-project/src/some%2Ftests/my-test.spec.js

Workaround

Per the documentation referenced above, adding "jsLoader": "require" to the Jasmine config (e.g., jasmine.json) allows tests to work again, I assume because it falls back to how Jasmine 3 worked.

sgravrock commented 2 years ago

Thanks for the report. This sort of thing will help inform whether and when we can stop having a separate CommonJS module loading path. (My guess is that we won't ever be able to get rid of it. Dynamic import is just too far from being a drop-in replacement for require.)

Out of curiosity, why do you have an encoded slash in a directory name?

chucknelson commented 2 years ago

@sgravrock

Out of curiosity, why do you have an encoded slash in a directory name?

This is a result of how the build pipeline works at my current employer - the pipeline checks out branches into paths that represent the branch name and URL encodes characters like slashes. So for a branch feature/something, the path it checks out files to is feature%2Fsomething.

We found a similar issue with eslint - so it seems like what we're doing is good for finding edge cases, but maybe we'll just change it to convert slashes to hyphens or something haha.

sgravrock commented 2 years ago

Thanks for the info.

I think jsLoader: "require" is the right solution, but we could probably produce a more helpful error message in this situation.

sgravrock commented 1 year ago

209, which was included in 5.1.0, fixed this.