jasmine / jasmine-npm

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

Export Loader #192

Closed mjeanroy closed 1 year ago

mjeanroy commented 2 years ago

Hi,

Problem

I wanted to implement watching & hot reload and I faced an issue:

Problem: even if it is possible by the API, it's not easy to override this option, as the loader cannot be imported by default as it is not in the "exports" field defined in package.json.

If you try to import it using: const Loader = require('jasmine/lib/loader.js'), you will get this error:

Error [ERR_PACKAGE_PATH_NOT_EXPORTED]: Package subpath './lib/loader' is not defined by "exports" in [redacter]/node_modules/jasmine/package.json
    at new NodeError (internal/errors.js:322:7)
    at throwExportsNotFound (internal/modules/esm/resolve.js:322:9)
    at packageExportsResolve (internal/modules/esm/resolve.js:545:3)
    at resolveExports (internal/modules/cjs/loader.js:450:36)
    at Function.Module._findPath (internal/modules/cjs/loader.js:490:31)
    at Function.Module._resolveFilename (internal/modules/cjs/loader.js:888:27)
    at Function.Module._load (internal/modules/cjs/loader.js:746:27)
    at Module.require (internal/modules/cjs/loader.js:974:19)
    at require (internal/modules/cjs/helpers.js:93:18)
    at createJasmine ([redacter]/index.js:65:18)

Workaround

The workaround I found is to require the Loader using an absolute path, for example:

const Jasmine = require('jasmine');
const jasmineDir = path.dirname(require.resolve('jasmine'));
const Loader = require(path.join(jasmineDir, 'loader.js'));
const jasmine = new Jasmine({
  loader: new Loader({
    requireShim: requireNoCache,
  }),
});

That's not great, and we can probably do better 😉

Suggestion

My suggestion would be to export Loader, exactly like the ConsoleReporter:

const path = require('path');
const util = require('util');
const glob = require('glob');
const Loader = require('./loader');
const ExitHandler = require('./exit_handler');
const ConsoleSpecFilter = require('./filters/console_spec_filter');

module.exports = Jasmine;
module.exports.ConsoleReporter = require('./reporters/console_reporter');
+ module.exports.Loader = Loader;

So, we'll be able to do:

const Jasmine = require('jasmine');
const jasmine = new Jasmine({
  loader: new Jasmine.Loader({
    requireShim: requireNoCache,
  }),
});

This way, it will be easier to implement hot-reloading, we just have to set the requireShim and we're good!

❓ Would you be ok with a PR to export the Loader?

sgravrock commented 2 years ago

The problem is that Loader isn't nearly stable enough to be part of the public API. In particular, there's been a lot of change recently to how it interacts with its dependencies. I expect more of that in the months to come as we learn about more special cases related to ES modules. If we made Loader public we'd likely to have to choose between doing much more frequent major releases and leaving ES module bugs un-fixed for years.

What we might consider doing is providing a way for users to supply their own loader, which would do the entire job that Loader currently does. That's not risk-free (there's a bit more to the implicit contract between Loader and Jasmine than might be apparent at first glance) but it seems a lot safer than exposing Loader.

sgravrock commented 1 year ago

Closing due to inactivity.