Improper file extensions used in packaging

[benmccann]

[REQUIRED] Describe your environment

• Operating System version: all
• Browser version: any
• Firebase SDK version: 9
• Firebase Product: messaging. probably others

[REQUIRED] Describe the problem

@Feiyang1 thanks for your efforts to improve packaging by adding exports to all package.json files (#5646). Unfortunately, I didn't catch this when reviewing that PR, but I'm afraid there's still an issue.

The file extensions are incompatible with Node. You must use .mjs for the ESM files (or set "type": "module" and use .cjs for the CJS files): https://nodejs.org/api/packages.html#determining-module-system

This bug was originally filed in the SvelteKit project (sveltejs/kit#3083), but it's really an issue with the way Firebase is packaged. Vite will try to load Firebase directly using the Node APIs and will fail. It may work if you use a bundler like Webpack because the bundlers will transform the code to be valid before trying to run it, but Vite will try to run the code directly during SSR and will fail as a result.

Steps to reproduce:

Try to load @firebase/messaging from a Node app without bundling and it will fail

Run npm install followed by npm run build in this project: https://github.com/sacrosanctic/test-static-messaging

You will get the error:

SyntaxError: Named export 'onBackgroundMessage' not found. The requested module '@firebase/messaging/sw' is a CommonJS module, which may not support all module.exports as named exports.

@firebase/messaging/sw is not CommonJS, but rather it's ESM. However, Node doesn't know that since there is neither "type": "module" nor an .mjs extension.

Relevant Code:

import "firebase/compat/messaging"

[argzdev]

Thanks for the report with the detailed explanation, @benmccann. We'll investigate this and check with feiyang or other engineers to see what we can do about this.

[charlotteliang]

@argzdev can you first see if you can reproduce this and assign back to me or feiyang to investigate?

[argzdev]

Hi @chliangGoogle, I was able to repro the issue with the given GitHub repro. I'll assign this case to you now, thanks!

[jacob-8]

jacob-8/sveltefirets#4 may be caused by this issue so I'll keep an eye on this.

[hsubox76]

Hi, just FYI, Feiyang has moved to another team so I can make any changes that are needed for this issue. Is messaging the only package affected? I'm asking because messaging is a little unusual, as it does not specifically provide "node" entry points in the "exports" field, for reasons given in the PR: #5646

As far as not having the mjs extension, I think what we did was we went with option 2 in https://nodejs.org/api/packages.html#determining-module-system where "Files ending in .js when the nearest parent package.json file contains a top-level "type" field with a value of "module"." We have a rollup plugin called emitModulePackageFile() that, during the build, creates a package.json with just {"type":"module"} in it, which it puts in the dist/esm folder.

If the other packages are working ok, then I think that method is ok and the problem with messaging might be that we don't have "node" entry points in "exports" which is due to the idb incompatibility mentioned in the PR. I think now that we have dropped IE support we may be able to upgrade our idb dependency version and maybe we can then add the Node ESM entry points for messaging, but that may be a moderate sized change as we use idb across multiple packages.

If the other packages aren't working either then I guess we need to get rid of this workaround and just use mjs extensions, I believe Feiyang initially avoided this in order to not have to build an additional bundle but if it's necessary we can do it of course.

[benmccann]

Thanks so much for taking a look! Messaging is the only one I'm aware of being affected. I haven't checked any of the other packages. I don't use Firebase myself, but am a maintainer on SvelteKit and the only report we've gotten thus far is about messaging.

Perhaps the issue lies with this particular directory that I'm getting an error message about:

node_modules/@firebase/messaging/sw

No code resides in that directory and it's instead resolved from the exports in the package.json of @firebase/messaging. I think that the package.json trick may not work in this case since the package.json for that directory isn't the nearest parent package.json of the actual code, but only a sort of virtually resolved entry point.

I had tweaked the package locally awhile back and then started getting errors about IDB, so it may be required to upgrade it as you mentioned to get this fully working. I hadn't really looked into that yet and was just trying to get this first error out of the way 😄

[hsubox76]

So I can change the extension of @firebase/messaging/sw/dist/index.sw.esm2017.js to mjs and it gets around that error, but immediately goes to the error stemming from our version of idb not being compatible with ESM. So I can't make a Node ESM version until we can upgrade idb which may take a while because it involves dropping IE support.

However, I can fix it by changing require in this line to node:

firebase-js-sdk/packages/messaging-compat/package.json

Line 13 in 4bd3a88

"require": "./dist/index.cjs.js",

I'm a little bit of a newbie when it comes to usage of the exports field, so is this okay? Basically, previously, the build tool would only be directed to that path if you were using Node and importing the package with require. If I change it to node, it will send any Node user to the CJS bundle of messaging-compat whether you are using require or import. Then transitively, messaging-compat will import the CJS bundle of messaging/sw which exists (as of #5884) instead of trying to get the MJS version of messaging/sw, which does not exist (and won't for a while, per the idb issue above).

I guess I'm just wondering if this will cause problems for any apps that expect only an ESM bundle when using import in Node and will break if they get a CJS bundle.

[benmccann]

That sounds probably okay and most likely better than the current situation. The main thing I'd be a little careful about is if you could end up with both a CJS and ESM version of the same file loaded and whether there would be any difficultly caused by that (e.g. two copies of any global variables). I'm not familiar with the Firebase codebase, so don't know if the change would be likely to cause any issue

[DellaBitta]

Hi @benmccann,

Checking in on this issue. It looks like some work was done to progress this issue back in January of 2022. Is this still an problem for you? Thanks!

[google-oss-bot]

Hey @benmccann. We need more information to resolve this issue but there hasn't been an update in 5 weekdays. I'm marking the issue as stale and if there are no new updates in the next 5 days I will close it automatically.

If you have more information that will help us get to the bottom of this, just add a comment!

[benmccann]

https://publint.dev/@firebase/messaging@0.12.4 is returning one suggestion for the packaging, but is no longer returning any errors, so this is likely fixed. Thanks!