Extending exports object detections

[guybedford]

Object expressons like:

    module.exports = {
      render: _render2['default'],
      shallow: _shallow2['default'],
      mount: _mount2['default'],
      ShallowWrapper: _ShallowWrapper2['default'],
      ReactWrapper: _ReactWrapper2['default'],
      configure: _configuration.merge,
      EnzymeAdapter: _EnzymeAdapter2['default']
    };

would still be beneficial to fully support reading their properties.

The challenge is doing this without a full parser that can't handle eg object expression / brace ambiguity details.

One idea is that it could be possible to use a , parser with no knowledge of object assign / expression ambiguity but purely based on matching the first , that happens at the same depth of brace tracking (including all brace types) which might be a way to dodge the harder parsing issues while supporting arbitrary expressions.

Will leave this open as a tracking issue for that kind of support.

[nicolo-ribaudo]

If this gets implemented, we must not forget to also consider template literals in depth tracking:

module.exports = {
  prop1: `${1,notExported}`,
};

[guybedford]

Right, it would effectively be a wrapped and nested implementation of the entire main parser.

[CxRes]

Would it be unreasonable to support this (even if it uses eval) under a flag, for those who want it?

Here is my use case: I am trying to monkey patch nw.js with node builtins (issue: nwjs/nw.js#7639). However, since they are defined as module.exports = {...}, the static lexer cannot decipher named exports. The way node itself handles this, i.e. providing esm versions of builtins written as cjs, is through C routines, a solution not suitable for user land. However if only I could have named exports I can implement this at practically zero cost.

[guybedford]

@CxRes can you share more details on the exact source patterns you need supported? It would be first worth assessing if this feature would really solve the problem.

I am hesitant to add new features at this late stage to this project though as well, since having highly variable compatibility for older versions of Node.js will be quite confusing...

[CxRes]

I am looking to add support for esm version of node built-ins (which are not available on the client/browser side of nwjs). For a pattern example, see the fs file here which is one of the larger and more complex APIs. What I intend to do is have a file with module.exports = require('fs') mapped using import maps to 'fs', so that I can import * from fs in my code.

The alternative is to transpile code to commonjs.

[guybedford]

So this issue wouldn't solve that. That's actually multiple separate issues to get to work:

1. Supporting module.exports = _identifier_ = {... object ...} form
2. Supporting getters / setters on object definition
3. Supporting logical and conditional expressions on object definitions

None of the above are at all related to this issue....

It would have been possible to really dive into these when we were first fleshing this out, but at this late stage in the day I think this is out of scope unfortunately.

You're likely better off just wrapping them in an ESM wrapper.

[CxRes]

I understand! However, given the maintenance burden of wrapping the entire node API (and it is unlikely that nwjs developers are themselves interested in fixing this), it looks like I would have to take the evil route and keep transpiling all my code to cjs (and maintain two versions of the same code).

I find it evil because by sticking to the legacy format, there is less incentive for others, like nwjs authors, to move ahead with esm. We are left in this mess for even longer, despite the herculean effort of good people like you in this space!

[guybedford]

Yes CommonJS will be around for another 10 years at least!

For what it is worth, an ESM wrapper is not that hard to do / not that much maintenance, see eg - https://github.com/jspm/jspm-core/blob/main/src-browser/fs.js.

[CxRes]

For what it is worth, an ESM wrapper is not that hard to do / not that much maintenance,

OMG!!! Maybe not for you, but for lesser mortals that is a bit too much. Not the process of creating such a library (which is straight forward), but the fact that you have to then keep on top of node API and that too different versions of it in perpetuity is borderline madness!!!

[guybedford]

Perhaps you could write a script that does Object.keys(require('fs')) then uses that to emit the wrapper automatically so you could just update it from time to time.

[CxRes]

Thanks!!!! After agonizing over it over the weekend, that is what I ended up implementing!

I was hoping to do this with a virtual file system at application but the I realized to map the paths I would need to start a server in the background, so gave up on that! Extra 25kB it is!

[cwadrupldijjit]

I can say that this specific issue would be wonderful to resolve. The current problem is with this itself.

The reason for it is that jest-runtime uses this package to do code lookup of CommonJS modules in order to help with resolution in the Node VMs that they use to run the tests in.

Unfortunately, when I was attempting to test code that imports (intentionally without mocking) the jsonwebtoken package, it's failing to find the exports assigned to an object being assigned to module.exports.

The code it's trying to parse looks like this:

module.exports = {
  decode: require('./decode'),
  verify: require('./verify'),
  sign: require('./sign'),
  JsonWebTokenError: require('./lib/JsonWebTokenError'),
  NotBeforeError: require('./lib/NotBeforeError'),
  TokenExpiredError: require('./lib/TokenExpiredError'),
};

Looking at the code and why it's not catching the requires that are in the object is because of the openTokenDepth being set to 1 since it's found within the object literal. That just messes with the reexports (if I'm reading that correctly). I imagine there likely needs to be a flag that says if it's the right position inside of an exports object assignment that can still allow for it to catch the requires as well as the object keys.

I can see one other possible limitation to that approach, and that's mostly because of reexports not being caught if they were assigned to a variable before they were added to the exports object, but I'm not sure how in scope that case is. I haven't pored over the code enough to understand everything that's happening, only the stuff that relates to parsing this specific case.

[cwadrupldijjit]

I failed to mention that the only thing it's catching as an export is the decode key. Everything else is ignored.

[cwadrupldijjit]

I think a similar problem was also happening to the mongoose package if you would like to know of another example.

[mvduin]

While I appreciate the complexity of supporting arbitrary expressions, it would be nice if it at least supported IDENTIFIER ':' REQUIRE idiom as shown in @cwadrupldijjit 's example as this is the most straightforward idiom to perform that kind of aggregation. I've used this idiom quite a few times myself in my own modules.

[Jack-Works]

I found this is not supported:

const result = require('cjs-module-lexer').parse(`
module.exports = {
    a: utils.a,
    b: utils.b,
    c: utils.c,
}
`)

This code is used by https://www.jsdelivr.com/package/npm/web3-utils, cjs-module-lexer can only detected export a, then the following input is dropped.

[Amerr]

This underlying issue is the blocker for upgrading to ESM. Is there any discussion or progress moving forward in this direction

@cwadrupldijjit have you find anything (even a hack) to solve this problem so that jest-runtime able to resolve named exports or conditional exports from a package.json file

[cwadrupldijjit]

I ended up moving most all of what I was working on to vitest since I also was moving much of my build processes to vite. One huge advantage with that is that it supports ESM out of the box. I also started using vite in Node-only based projects, not to build a front-end but in library mode and specifically only to run vitest. Some advantages I encountered that made me love vitest over jest:

• Built-in ESM support
• Built-in TS support
• Executed code ends up almost identical to the original TS code since the latest versions of Node support nearly all of the current spec for ECMAScript; almost all that is removed or changed is the types themselves
• Configuration for vitest doesn't take long and has everything included in it that I wanted from jest

I also dabbled in more recent projects using the built-in Node test runner, which though it may not be considered stable, it definitely includes most of what I need in a test runner. It is not without its issues, but I got it working to a good enough extent for my setup in my side projects. That was written in TS and it seems like some reporting and coverage parts that I attempted to use didn't really work.

[Amerr]

@cwadrupldijjit Thanks for your input, unfortunately, our app is a web app. So it is going to be a long journey toward ESM conversion.

[cwadrupldijjit]

I'm sorry if it sounded like I had to use vite for a Node app--vite is supposed to be used to bundle a web application. vitest doesn't have to be tied to a web app and that's what I was talking about. With me, I configured vitest to run as if it was running like jest does and it was a direct replacement. It doesn't have the same dependencies as jest does, which means that the bug preventing jest from reading ESM/TS like it should.

However, I can think of a few alternatives in the case that you can't jump off of jest:

• Fork this repo and modify it to work like you need; you can potentially refer to a separate package registry that resolves this package as the forked version instead; big downsides include having to maintain a fork should anything change upstream
• If you're using a preprocessor like TS, make sure that you're not setting the compiler option in the tsconfig to ESM, which I believe means that the processor should compile it like it should; this might mean that you need to have a separate tsconfig specifically for tests or at the very least might require a janky change in your configuration to support it (like removing the "type": "module" configuration in the package.json just for the tests)

In my case, I couldn't find a great way to work around this issue with jest and just gave up on it when I found out that vitest was a much better setup experience. If you would like me to, I could post an example of a decent working example of vitest replacing the jest setup.