joshmossas / event-source-plus

A more configurable EventSource implementation that works in browsers, node, and workers
MIT License
20 stars 0 forks source link

Bug: jest-environment-jsdom cannot resolve dependencies #1

Open FC-jan opened 2 months ago

FC-jan commented 2 months ago

Hi there :)

thanks a lot for your library! when I was unhappy with our old choice, this one seems likes a great replacement! I'd have one question regarding jest testability though.

After switching, I had to introduce a mock like the following and actively mock it in failing tests:

const EventSourcePlus = jest.fn().mockImplementation(() => {
  return {
    listen: (url, options) => {},
  };
});
module.exports = EventSourcePlus;

Which solved my test failure - but made me think, did I maybe miss sth wrt to testing support? I think it would be great to increase the mock's functionality for more in-depth tests :)

Best

edit: forgot to post the initial failure I faced:

    /srv/www/node_modules/ofetch/dist/index.mjs:1
    ({"Object.<anonymous>":function(module,exports,require,__dirname,__filename,jest){import { c as createFetch } from './shared/ofetch.37386b05.mjs';
                                                                                      ^^^^^^

    SyntaxError: Cannot use import statement outside a module
joshmossas commented 2 months ago

Hey @FC-jan

Thanks for the bug report. I've been using vitest for the past few years so jest support is a blindspot for me. Could you provide a copy of your jest config and tsconfig.json if you are making use of them?

joshmossas commented 2 months ago

So I decided to mess around with it and I was able get Jest working. My babel.config.js looked like this.

module.exports = {
    presets: [
        ["@babel/preset-env", { targets: { node: "current" } }],
        "@babel/preset-typescript",
    ],
};

And my tsconfig.json looked like this

{
    "compilerOptions": {
        "target": "es2016",
        "module": "commonjs",
        "esModuleInterop": true,
        "forceConsistentCasingInFileNames": true,
        "strict": true,
        "skipLibCheck": true,
        "types": ["jest"]
    }
}

So most likely there's prolly just some config tweaks that need to be made to get it running in your project. Based on the error you shared I think jest is trying to import the esm files instead of the commonjs files.

joshmossas commented 2 months ago

@FC-jan I'm going to go ahead and close this since it's likely a jest config issue. If you have additional details feel free to post them here and I can try to give some guidance.

FC-jan commented 2 months ago

Hey @joshmossas, sorry for the delay and thanks for looking into it :) I think I was able to isolate my issue when setting up a reproduction: https://github.com/FC-jan/event-source-plus-jest-reproduction In this example repo, I'm able to fix it by switching jest.config.js' testEnvironment to 'node'. Sadly, my actual use case is within a big symfony react application, which necessitates jsdom as test environment (it offers test replacements for document global and the like).

That's why I also wasn't able to just upload my files on the spot, they needed reduction :)

(PS: Also tried babel and tsconfig closer to your suggestion - had issues with that in my project, and iirc, the testEnvironment issue persisted independently. Can clean it up or bring it closer if it's of help. Also gave you repo-permissions if I should test anything :)

joshmossas commented 1 month ago

So I've spent quite a bit of time debugging this and the issue seems to be caused by a bug in jest-environment-jsdom. For some reason when jest has the environment set to jsdom it does not resolve the imports correctly. I have tried multiple configuration including setting up a custom transformer but it's quickly turning into a rabbit hole, so I'm inclined just to leave it be for the time being in hopes that the Jest team eventually fixes the issue.

I'll leave this open for visibility. Maybe down the line I might take another crack at it or dig through the jest-environment-jsdom source code to see if I can find the source of the issue. Cheers!

FC-jan commented 1 month ago

First of all, thanks for your dedication on the issue 🙏

I wouldn't expect a fix on jsdom side sadly, afaik their setup is like this for the reason to being close to transpiled output without supporting more modern language features.

Meanwhile, I realized my tests didn't even get to use the mock I mentioned initially yet. It evolved from said snippet to this one now:

jest.mock('event-source-plus', () => {
  return {
    EventSourcePlus: jest.fn().mockImplementation(function EventSourcePlus() {
      return {
        listen: (url, options) => {},
      };
    }),
  };
});

I agree though that it's not worth to get into that rabbit hole for now. If I find additional ways to improve testing in jsdom environment, I'll note my findings here as well. I'm especially curious to mock some messages to test their outcome, which I believe should also be possible via a (similar) mock, maybe just exposing a setTimeout or sth. like that.

Thanks for your dedication again, for this lib (using it on prod successfully, big improvement over its predecessor : ), and for leaving the issue open for visibility. Also hope that it maybe yields further insight without getting into the rabbit hole :)

All the best