Jest does not work with Firebase out of the box

[levino]

Currently there are ".js" files generated which are "es modules". Since there is no "type": "module" in the package.json, this is simply illegal as far as I can tell. It creates issues when trying to load firebase with jest and jsdom.

Either we have to make this a module or always use .mjs for all modules.

[google-oss-bot]

I couldn't figure out how to label this issue, so I've labeled it for a human to triage. Hang tight.

[hsubox76]

If I understand correctly, these rules (the extension must be mjs or there must be type: "module" in the package.json) apply to Node ESM module resolution specifically. In the file you linked above, default is supposed to point to the browser bundle, while we do in fact point to an mjs file for node.import, which should be the relevant case where these rules apply.

I think that Jest/JSDOM runs in Node but for some reason, the build config prefers to pull in the browser bundle. I'm sure there's a reason for this but if the build system is set to follow Node rules there's probably going to be problems if it's pointed at a browser bundle. If there's any way to change the resolver setting in your build config so it prefers the node.import field over the default field, that's probably the best way to solve this. I think that a Node build toolchain expecting Node conventions should be using the Node path and I think it might not be the best idea to change the browser bundle to conform to Node conventions.

[levino]

As far as I understand, jest has its own module resolution and loader. It needs to be able to mock modules, so it needs to somehow overwrite import and require. When the testEnvironment is jsdom, jest mocks a browser environment with window and document etc. So it should be fine to then also "run browser code". This is why jest when run in jsdom-mode will load "browser" files and this is fine. However it does not expect these files to be es modules. It only expects them to be es modules, when they declare, that they are (by the usage of "type": "module") (x)or by declaring it for the files (by giving them the .mjs extension). Is there a place where it says "it is fine to publish an npm package that contains esmodules without setting "type":"module" in the package.json"? I could not find one, but maybe I also overlooked something. Also if we just look at this from the other side: What is the downside of using the .mjs extension as I suggest in #6748 ?

[hsubox76]

Looking around, it seems the .mjs extension causes problems in a number of environments, such as older versions of build tools, or when they are in a project combined with common js dependencies.

The rules cited above (mjs or "type:module") are rules for Node ES modules specifically, not rules for all ES modules. Unfortunately there are is a lot of different build tooling out there for browser bundles and some of it breaks if you don't follow those rules, and some of it breaks if you do.

I'm not sure there is a really good solution to "I want it to resolve following Node rules, but I want the resolved module to behave like browser code" that doesn't break somebody.

I'll try to do some research, if this is very standardized now and there are only a few outliers that would be broken maybe we could make it a breaking change in 10.x similar to us moving to ES2017 as the default bundle in 9.x.

[hsubox76]

Looking more closely, it looks like the @firebase/* packages (@firebase/app, @firebase/auth, etc.) inject a package.json that contains only "{type: module}" into a dist/esm subdirectory where all the esm bundles go and this hasn't been causing any problems for older build tools as far as I can tell.

So we might be able to do the same in the firebase packages. Short explanation: there's a main package, firebase, where all the paths (firebase/app etc.) are just entry points that point to their matching @firebase/* packages. The @firebase packages use the "type:module" trick for all ESM bundles, but the firebase/* entry points don't. Perhaps if Jest/JSDOM and other Nodelike resolvers are getting stuck at the entry point, we can add the same one-line package.json to the entry points and put the ESM files in subdirs.

[hsubox76]

I'm working on adding "type:module" to the files, as you can see in the linked PR but in the meantime I stumbled across this old issue and wondered if this solution will work for you as a workaround for now: #6417 (comment) You didn't say what specific error you were running into so this might be something completely different, but thought I would link it just in case it helped you get unblocked until this can be addressed in the packaging.

[levino]

levino/react-firebase-hooks#3 The "import" errors have gone, but now firestore gets confused and things go south.

And just as a remark: As you can see from the list of "bad dependencies", it looks like firebase is one of the very few that are incorrectly configured to work with jest.

[levino]

Looking around, it seems the .mjs extension causes problems in a number of environments, such as older versions of build tools, or when they are in a project combined with common js dependencies.

Would you mind sharing the sources for this? Would it be possible to add ci tests for these compatibility issues so one can find a solution to all problems without breaking anything? Also often these issues rather arise from consumers misconfiguration of their build tools than from the publisher using a wrong file extension. So maybe these issues where fixed in the wrong place...

[levino]

And just as a bit of context: I want to write tests for firebase hooks which interact with a firebase emulator back end. To test the hooks I tried using renderHook from @testing-library/react. This renderHook needs jest to run in jsdom (idk why, but also kinda makes sense). However when I use the renderHook from @testing-librar/react-hooks it works with jest in node since the implementation is different. That was my workaround which is quite shaky since any time soon I might need to use something from @testing-library that requires jsdom. Then I am doomed.

[hsubox76]

As mentioned, I am changing the entry points to conform to Node ESM standards using the "type: module" method (the second option here), so there isn't a need for the mjs extension. PR: #6826 I think this is the maximally backwards compatible solution that also fully conforms to Node ESM standards.

[hsubox76]

I created a Next.js project with Jest and Firebase and ran a basic test out of the box to get the error I assume you are getting (you didn't mention what specific error you were getting but you mentioned "import" errors in a comment):

    Details:

    /Users/chholland/repos/next-project/node_modules/firebase/app/dist/esm/index.esm.mjs:1
    ({"Object.<anonymous>":function(module,exports,require,__dirname,__filename,jest){import { registerVersion } from '@firebase/app';
                                                                                      ^^^^^^

    SyntaxError: Cannot use import statement outside a module

I tested it out with my new PR and it did not fix that error. Changing the extension to mjs and changing the package.json exports field to point to the newly named mjs file also did not fix it, however (in my Next/Jest setup). Did changing it to mjs work for you? Can you tell me more about your setup? You said you're using Jest, and you mentioned react-firebase-hooks. Is that the repo you are working on?

I guess what I'd like to do is create a reproduction the error locally so I can see it, so I can test a number of possible options to see if they make the error go away. So I'd like to first make sure I'm reproducing the same error you are getting.

Edit: I followed the instructions here: https://jestjs.io/docs/ecmascript-modules and I'm getting this error:

    Must use import to load ES Module: /Users/chholland/repos/next-project/node_modules/firebase/app/dist/esm/index.esm.mjs

Which again, is with already using the mjs extension.

[levino]

Thank you very much for spending so much time on this @hsubox76 and sorry for my late response.

I reproduce the original error here or here. This example is using the version 9.14.0 of firebase.

How can I use your code? Has it been released as a x.x.x-beta version or something? Unfortunately it is not just one package, but a full namespace which I find a little tricky to build locally (I mean that is the easy part) and then correctly link it into another repo. So if you could be so kind update my branch to use your fixed version of firebase, I would be happy to investigate further.

[levino]

Also a suggestion: Before working any more on fixes, I would suggest to somehow write some kind of integration test (build firebase and run some test with jest in jsdom mode which creates this error) that reproduces the issue I experience in the ci scripts of firebase. If that is not added at least as a regression test, tomorrow someone might rollback or just change again your changes and until this again hits downstream users, it might take a while and then be difficult to figure out.

I will give this idea a spin, once I have some time.

[hsubox76]

I'm going to keep looking into this regardless but is your goal to run tests in Jest using ES module imports or is it to just run tests in Jest successfully at all?

Because it seems like ES module support in Jest is currently experimental and requires a flag and a number of settings. https://jestjs.io/docs/ecmascript-modules

Additionally, searching around the web, it looks like people are having a lot of trouble setting it up and it tends to be very finicky.

The workaround I referred to above in another PR (direct link here) is what a lot of Jest/Firebase users are using to just make it work in Jest (not caring about using ES modules specifically). Looking closely, what it actually does is delete the entire exports field and module field out of every firebase package.json, which is sort of a brute force way of leaving only the main field and forcing Jest to resolve to the CJS bundle.

This should solve your initial problem of the tests not starting up at all in Jest. The subsequent problem you're having with Firestore erroring is a separate issue which probably won't be solved with any kind of module resolution fixes. It might be related to this bug here? #6658

The workaround does seem really messy to me and there has to be a cleaner way of forcing Jest to use CJS bundles at the very least, so I don't think asking every Jest user to use that workaround is something to be satisfied with. I'm going to keep looking into some kind of less cumbersome solution for the default case for Jest users.

Oh, and if you want to verify for yourself if the mjs solution works or doesn't, the quickest way is to go into your project into node_modules/firebase/package.json and change the exports.firestore.default field to an mjs extension (just as you did in your PR), then go into node_modules/@firebase/firestore/dist/esm and rename index.esm2017.js to index.esm2017.mjs, and then run your test again. It didn't work when I tried it in a clone of your project.

[hsubox76]

FYI I changed connectFirestoreEmulator(db, 'localhost', 8080); in your test/firebase.ts file to connectFirestoreEmulator(db, '127.0.0.1', 8080); (also had to export FIRESTORE_EMULATOR_HOST=127.0.0.1:8080 beforehand) and that Firestore error went away. Just saw individual test errors after that. But if you continue to have issues with the emulator, check that linked issue or open a new one if it's a different issue.

[levino]

I have no issues with the emulator. All tests pass without jsdom in the main branch of the react-firebase-hooks package which my PR is based on.

In order to emulate a firebase version with your changes locally like you suggest I have to change the files in dozens of subpackages of @firebase. The resolver jumps around between these packages like crazy, since it is monorepo with a lot of packages. The easiest way would be if you could publish all these packages with a *-beta tag or something on npmjs.org. Changing the file extensions etc. in all these subpackages by hand is too tedious and also too error prone. Linking the whole workspace locally via yarn link could work, but I had issues, last time I tried (with my changes). I can try with your commit again however.

Also this issue is not super urgent for me any more so maybe we could just wait for the next release of firebase which I guess would contain your changes, right?

[levino]

I'm going to keep looking into this regardless but is your goal to run tests in Jest using ES module imports or is it to just run tests in Jest successfully at all?

My goal is to load firebase in javascript code, run via jest with testEnvironment: "jsdom". My tests are fine and pass with testEnvironment: "node". The PR just serves as a quick and easy "minimal example". I guess it could be made even easier by just having an empty repo with a single index.ts file importing firebase and no test or a very simple hello world. test which actually does nothing. If testEnvironment: "jsdom", the firebase import will crash the jest thread. In this sense firebase is a broken package with illegal usage of esmodule syntax without proper delcaration via the package.jsons "type" field and not usable in certain projects (projects that use jest to headlessly test web front end code, which should be rather common). The only one I stumbled upon so far.

I guess why not more people have this issue is that usually, people find it too difficult to set up jest and the emulators so that they can run integration tests against it. It is somewhat clunky and quirky but doable and quite necessary for proper development with firebase imo.

[hsubox76]

The changes I made in the release won't work for your use case. They adjust some entry points to follow Node ESM rules which isn't the problem with Jest.

The correct fix for Jest, in standard mode, without using the experimental ES modules (which you aren't, you'd have to set the flag) is to change the Jest resolver as described in that link. You can actually make the jest.resolver.js file even shorter:

// jest.resolver.js
module.exports = (path, options) => {
  // Call the defaultResolver, so we leverage its cache, error handling, etc.
  return options.defaultResolver(path, {
    ...options,
    conditions: ['node'].concat(options.conditions)
  });
};

That's because the problem is that Jest's resolver by default is set up to prioritize exports fields ['require', 'default', 'browser'] in that order. If it doesn't find any exports field at all, it then looks at module at the top-level and then main. It clearly wants the CJS bundle when a CJS bundle is available.

However, while Firebase packages do indeed all have a require field in the exports field, they are nested under a node field (example), and I guess the resolver does not check inside nested fields. Or maybe it only does after it checks the top-level fields. In any case, the second item in the array is default, which it's able to find at the top level, which points to the ESM bundle, which Jest doesn't like.

I have to think about who should fix this, but I feel like Jest is incorrect here. Nesting require and import conditions inside a node condition is pretty standard? Node.js docs specifically suggest it: https://nodejs.org/api/packages.html#nested-conditions I think Jest should recognize it? The fix would be for Jest to change its exports conditions to add "node" at the beginning so it's ['node', 'require', 'default', 'browser'] (that's what my resolver fix above does, it overrides Jest's resolver config to this).

In order for us (the Firebase SDK) to "fix it" without changing Jest, we'd have to create a duplicate "require" field outside the "node" object for every package, which seems kind of hacky.

Anyway I guess I may look around the Jest repo to see if this has been discussed and maybe bring it up if it hasn't been.

For a workaround, I think most users can add that jest.resolver.js file I pasted above, and then just add one line to jest.config.ts, and that's not out-of-the-box but it should work.

If you don't mind, I'm going to change the issue title to "Jest does not work with Firebase out of the box", for better discoverability and such, by other people having Jest problems. Let me know if that is not an accurate summary of the main problem we're trying to fix here.

[levino]

The jest issue is a symptom of the incorrect way to build this package, this is why I did not mention jest in the original issue descrioption.

You can only have a esmodule js file if you say type: "module" in the package.json. Also this is the problem, not this. Either the default file is a cjs module, or it is a esm AND it has type: "module" in the package.json. You have to choose but cannot mix the two patterns. At least this is what I find on the internet. If you have other sources please share them with me. This is also quite confusing for me and I am just trying to make sense out of this. I also recommend dropping all these special extensions like es2016.js etc. Imo they are just noise. Even more so if they live in folders like /es2016/index.es2016.js now. I would use .cjs, .js, .mjs exclusively.

What I can say is: firebase should have the ambition to be compatible with something like jest (1.3 mio downloads vs 19 mio downloads) without having to jump through hoops on the user end. This would only be acceptable, if very good reasons prevent it, but so far I cannot see any. It would really help if the backwards compatibility issues could be linked here so we can see whether they still are a problem. Also I would rather correctly support jest than browserify.

[levino]

I created https://github.com/levino/firebase-js-sdk/tree/feature/use-yarn-berry which is based on your changes but uses yarn@berry. What you can do is now, you check out https://github.com/levino/react-firebase-hooks/tree/bbca0e19d504a47794d1747b3fe008854af9850f which also uses yarn@berry. So maybe you have now a folder structure like:

coding/firebase-js-sdk
coding/react-firebase-hooks

Then you go to coding/react-firebase-hooks and run yarn link ../firebase-js-sdk --all which makes react-firebase-hooks use the local code (which includes your changes from #6826). When I then run the tests (which are configured to run with testEnvironment: "jsdom") it works!!. So #6826 fixes the issue as I expected but needed to verify.

What remains is adding a regression test.

Also please note that I had to configure the firestore a bit with:

{
  experimentalAutoDetectLongPolling: true,
  useFetchStreams: false,
}

because if I did not to this, the tests would simply time out. I guess the firebase js code for browser tries to use some fancy feature that is not supported correctly in jsdom and also I had to use 127.0.0.1 instead of localhost for the connection, because if not firestore would try to use ipv6 and then some errors pop up here. But these changes are quite acceptable.

[hsubox76]

#6826 has been released with 9.15.0 and I tried it in your project and it doesn't seem to fix the issue, but if it's working for you I guess I'll close this and you can reply if it's still not working.