Skip to content
This repository has been archived by the owner on Jan 6, 2024. It is now read-only.

Doesn't Appear to work with Top level node Exports #11

Closed
dairyisscary opened this issue Jan 28, 2022 · 6 comments
Closed

Doesn't Appear to work with Top level node Exports #11

dairyisscary opened this issue Jan 28, 2022 · 6 comments

Comments

@dairyisscary
Copy link

Hello!

Thanks for the great package! However, I ran into an issue with packages that use a exports for the "top-level" node modules export. This package only appears to work if the import request is for a submodule like import x from "my-node-package/submodule";. it is possible for a package to use the exports field for root like import x from "my-node-package";.

For an example of this, see solid js which uses exports quite extensively to support different code paths in the server vs the browser.

https://github.com/solidjs/solid/blob/main/packages/solid/package.json#L37

In other words, jest-node-exports-resolver seems to work correctly when importing solid-js/web but not solid-js. i was able to work around this by making two changes to the resolver:

    if (!request.startsWith("@")) {
      if (length > 1) {
        packageName = pkgPathParts[0];
        submoduleName = `./${pkgPathParts.slice(1).join("/")}`;
      } else { // <- added this else
        packageName = request;
        submoduleName = ".";
      }
    } else if (length > 2) {
      packageName = pkgPathParts.slice(0, 2).join("/");
      submoduleName = `./${pkgPathParts.slice(2).join("/")}`; // <- I'm not sure if this line works for @namepspace/root packages but i didn't have any packages lie that to test
    } 

i also needed to add a check on conditions since this change alone broke for the package expect (which has a conditions of undefined) as far as i can tell, this is a good change regardless since we can skip the resolveExport function in more cases

        if (typeof exportValue === "string")
          targetFilePath = exportKey.endsWith("*")
            ? exportValue.replace(/\*/, submoduleName.slice(exportKey.length - 1))
            : exportValue;
        else if (conditions && exportValue !== null && typeof exportValue === "object") { // <- Modified this line
          function resolveExport(exportValue, prevKeys) {

I have no idea if these changes are good in general. i'd be happy to make a PR if you think this is good idea.

@piranna
Copy link
Collaborator

piranna commented Jan 28, 2022

For an example of this, see solid js which uses exports quite extensively to support different code paths in the server vs the browser.

solidjs/solid@main/packages/solid/package.json#L37

That package.json file is using node condition, that's not supported by jest. That's the reason why it's not working for you. In fact, I have another PR open to add support to it. It's not a problem of jest-node-exports-resolver.

@dairyisscary
Copy link
Author

oh right! i failed to mention, i actually desire the browser conditions (since i want to test a "browser" like/client env). I've already made my own test environment with browser. even with this env, though, jest-node-exports-resolver still doesn't resolve the "." exports.

To be clear, i have something like this:

const JsdomEnv = require("jest-environment-jsdom");

module.exports = class MyEnv extends JsdomEnv {
  exportConditions() {
    return ["browser"];
  }
};

and I'm using this env in jest's testEnviornment configuration.

console logging in jest-node-exports-resolver produces correct ["browser"] as conditions value for the solid-js request.

@piranna
Copy link
Collaborator

piranna commented Jan 28, 2022

Do you have a test case that we can replicate?

@dairyisscary
Copy link
Author

@piranna good idea. i should have just done this from the get go.

https://github.com/dairyisscary/jest-enviorn-root

$ node --version 
v16.13.0
$ npm install
$ npm test

if the test fails, that means that jest is using the server version of solid-js (./dist/server.js), when i want the browser version(./dist/solid.js). the test it will pass if it imports the correct version -- i verified that by monkey patching my local installation of jest-node-exports-resolver. hope this helps.

@piranna
Copy link
Collaborator

piranna commented Jan 29, 2022

I've published a new version 1.1.5, can you check if it works?

@dairyisscary
Copy link
Author

@piranna its working swimmingly! thank you so much for your efforts. I consider this issue closed.

(side note, i was able to pull your 1.1.5 from npm but it looks like you've forgotten to push your changes to github too -- just a heads up)

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants