Skip to content

v9 export issues with Vite #5140

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Closed
abdo643-HULK opened this issue Jul 10, 2021 · 23 comments
Closed

v9 export issues with Vite #5140

abdo643-HULK opened this issue Jul 10, 2021 · 23 comments
Assignees
Milestone

Comments

@abdo643-HULK
Copy link

[REQUIRED] Describe your environment
Operating System version: macOS 11
Browser version: Safari 14
Firebase SDK version: 9
Firebase Product: all

[REQUIRED] Describe the problem

I am working with SvelteKit which uses Vite for bundling and I am not sure if it's the firebase "export * from '@firebase/app';" or if it's Vite. But when importing firebase like this "import { initializeApp } from 'firebase/app';" Vite says it's a commonjs package. If it's a dev depencency and trying to build it reports as followed:

> Named export 'initializeApp' not found. The requested module 'firebase/app' is a CommonJS module, which may not support all module.exports as named exports.
CommonJS modules can always be imported via the default export, for example using:

import pkg from 'firebase/app';
const { initializeApp } = pkg;

For the dev server it works fine. When installing firebase as a normal dependency and running build everything works fine but when trying to run the dev server this error pops up:

12:45:52 [vite] Error when evaluating SSR module /node_modules/firebase/app/dist/index.cjs.js:
ReferenceError: exports is not defined
    at eval (/node_modules/firebase/app/dist/index.cjs.js:5:23)
    at instantiateModule (/Users/abdo/Downloads/svelte-firebase-error-master/node_modules/vite/dist/node/chunks/dep-cc49d7be.js:72639:166)
exports is not defined
ReferenceError: exports is not defined
    at eval (/node_modules/firebase/app/dist/index.cjs.js:5:23)
    at instantiateModule (/Users/abdo/Downloads/svelte-firebase-error-master/node_modules/vite/dist/node/chunks/dep-cc49d7be.js:72639:166)

Like I said I am not sure if it's because of the all export or if it's Vite but changing the "import { initializeApp } from 'firebase/app';" to "import { initializeApp } from '@firebase/app';" doesn't throw an error in both cases.

@google-oss-bot
Copy link
Contributor

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

@haoqunjiang
Copy link

haoqunjiang commented Jul 10, 2021

Oops, my previous comment is also out-of-date.

The real issue here is that Vite failed to automatically externalize firebase v9 during SSR.


For the Vite issue here, because it can be worked around by configuring ssr.externals, I'd say it's more like an inconvenience than a bug.
It's the "automated SSR externalization heuristics" that lead to the wrong outputs in the firebase case.
But these heuristics aren't guaranteed to always work. That's exactly why manual options like ssr.externals exist.

I'll try to improve the heuristics anyway.

@haoqunjiang
Copy link

Workaround:

svelte.config.js:

/** @type {import('@sveltejs/kit').Config} */
const config = {
	kit: {
		// hydrate the <div id="svelte"> element in src/app.html
		target: '#svelte',
		vite: {
			ssr: {
				external: ['firebase']
			}
		}
	},
};

export default config;

@Eugeniegrandit
Copy link

This workaround works perfectly well, thank you @sodatea!

@abdo643-HULK
Copy link
Author

Workaround:

svelte.config.js:

/** @type {import('@sveltejs/kit').Config} */
const config = {
	kit: {
		// hydrate the <div id="svelte"> element in src/app.html
		target: '#svelte',
		vite: {
			ssr: {
				external: ['firebase']
			}
		}
	},
};

export default config;

It works on the dev server but it throws the commonjs error when you try to build the project

@akauppi
Copy link

akauppi commented Jul 11, 2021

It works on the dev server but it throws the commonjs error when you try to build the project

A long while back, i decided against Vite for production build and went with Rollup, instead. Things likely have changed, but that’s a suitable and proven workflow. Note that Vite uses Rollup underneath, so it’s just bypassing one layer. The downside? More custom configuration is needed.

Watching this space. If you manage to get Firebase modular + Vite builds working, I might reconsider my choice.

@abdo643-HULK
Copy link
Author

It works on the dev server but it throws the commonjs error when you try to build the project

A long while back, i decided against Vite for production build and went with Rollup, instead. Things likely have changed, but that’s a suitable and proven workflow. Note that Vite uses Rollup underneath, so it’s just bypassing one layer. The downside? More custom configuration is needed.

Watching this space. If you manage to get Firebase modular + Vite builds working, I might reconsider my choice.

It works but you have to change all "import {} from 'firebase/..'" to "import {} from '@firebase/..'". Internally the first import just does a "export * from '@firebase/..'".

@haoqunjiang
Copy link

haoqunjiang commented Jul 12, 2021

It works on the dev server but it throws the commonjs error when you try to build the project

Well, this one gets a little bit tricky.
It's not a Vite issue in this case.
SvelteKit emits ES modules for SSR outputs (by default Vite would output CommonJS for SSR targets for best compatibility). So it expects all external dependencies to support ESM exports. However, firebase v9 have the following lines in its exports field:

"./app": {
 "node": "./app/dist/index.cjs.js",
 "default": "./app/dist/index.esm.js"
}

which means, in Node firebase/app always redirects to firebase/app/dist/index.cjs.js, and in other environments, it's firebase/app/dist/index.esm.js.

Apparently, the CJS bundle isn't statically analyzable, thus no named exports available to the users.

@haoqunjiang
Copy link

I don't know if SvelteKit supports emitting CJS bundles for SSR. If it does, the abovementioned workaround should suffice.

If not, I think the issue needs to be addressed in firebase.

@haoqunjiang
Copy link

haoqunjiang commented Jul 12, 2021

For the firebase team, there are several ways to fix this issue:

  1. The easy fix: pin the Rollup version to ~2.38 when building the firebase package.
    It's the same problem as Cant import on node using native ESModules (.mjs) (3.0.6) vuejs/core#3332, which is actually a bug in cjs-module-lexer. Pinning the Rollup version can circumvent it. After this change the CJS exports would become statically analyzable by Node.js, so that named exports will be available when imported from an ES module.

  2. The more robust fix: instead of only supporting node and default, it should add import and require conditions too:

"./app": {
  "node": {
    "import": "./app/dist/index.mjs",
    "require": "./app/dist/index.cjs"
  }
  "default": "./app/dist/index.mjs"
}

Note that the extensions need to be changed accordingly.

@Feiyang1
Copy link
Member

First, thanks for the report. Some context - Firebase has different implementations for Nodejs and Browser, so we have separate bundles for them. Currently, we only provide cjs bundle for Nodejs, and only esm bundle for Browser.

@sodatea supporting import and require conditions in firebase package sounds reasonable, and is easy to do. We probably need to do the same for the scoped packages, i.e. @firebase/app, (firebase is just a wrapper that reexports everything from scoped packages). I will test it out, but do you know offhand if that's the case?

@akauppi
Copy link

akauppi commented Jul 12, 2021

@Feiyang1 Have you considered limiting thefirebase package’s role to only providing the version? I.e. one would use it in the package.json and change logs - as is now.

But for imports, you could document that people should use the scoped packages directly.

This is the way I’ve grown to use them, and it’s mentioned in this issue. But it’s not in the docs.

@haoqunjiang
Copy link

@Feiyang1 I just took a look at @firebase/app and @firebase/analytics, I don't think they need the exports fields yet.

What's breaking currently is that Node.js failed to convert CommonJS re-exports to ES module re-exports (due to the bug linked above).
Those packages without re-exports still can be used in both module systems.
Adding native ES entries to them is great, but not urgent.

@jthegedus
Copy link

I would warn that the new Firebase v9 SDK should not be used in SSR contexts, that should be performed by the Firebase Admin SDK (which also has a new modular version). While the v9 Client SDK certainly supports execution in Node.js environments, this is intended for use in Electron style apps, not server environments. Any server environment should use the Admin SDK and SSR falls under this category.

Sharing here from the linked SvelteKit issue.

@Feiyang1
Copy link
Member

@sodatea Thanks! I think I have a good understanding of the issue now. The issue should really be addressed in cjs-module-lexer as you pointed out here, but this whole cjs/esm interop story feels really fragile, so I think it's a good practice for libraries to provide both cjs and esm for Nodejs.

The downside is the build becomes more complicated in order to address dual package hazard. For now, I'm only changing the firebase package to support dual packages because it addresses the problem, and doesn't require changing our build process.

@akauppi I can't say I haven't thought about it, but it's a really big change from the documentation perspective, and potentially confusing for some developers (you import different packages than what you installed). And for the vast majority of, if not all, use cases, there shouldn't be any difference in using firebase vs @firebase, so I think it's unlikely to happen.

@jthegedus Thanks for sharing, but I'm afraid the info is misleading. The Firebase JS SDK can be totally used for SSR. In fact, many people are doing that today and AngularFire has been using the JS SDK for SSR for a long time, and it's not changing with v9.

If there is any issue with SSR with the v9 SDK, please open issues with us, and we will try to fix them.

@akauppi
Copy link

akauppi commented Jul 17, 2021

@Feiyang1 I’ve gathered some ES packaging experience with Rollup. Would you like me to try a review/revise of the build setup for the Admin SDK? I’m thinking ES first and creating CJS from those sources, via Rollup.

I have time for this this week, but would like to sync with Firebase before and after the task, to understand the aims of the product.

@jthegedus
Copy link

jthegedus commented Jul 18, 2021

@jthegedus Thanks for sharing, but I'm afraid the info is misleading. The Firebase JS SDK can be totally used for SSR. In fact, many people are doing that today and AngularFire has been using the JS SDK for SSR for a long time, and it's not changing with v9.

The topic is more complex than your statements imply and simply suggesting it is fine without discussing the implications is more misleading in my opinion.

I do not use the AngularFire library, but I would image it handles the specific challenges for which SSR becomes difficult that I am raising. For people not using the AngularFire library and just using the base SDKs, the interactions of SSR, Authentication and tokens, Security Rules & Firestore queries should be understood before simply using the Client SDK in SSR context.

@Feiyang1
Copy link
Member

@akauppi Thanks for the offer, I'd start with chatting with @hiranya911 who is the maintainer of the Admin SDK.

@jthegedus These are all valid points regarding the caveats for SSR. I just wanted to point out that the blanket statement the new Firebase v9 SDK should not be used in SSR contexts is inaccurate, and I don't want to leave people with the wrong impression.

You can use either the JS SDK or the Admin SDK to do SSR with pros and cons(limitations) respectively. Which one to use is really based on your specific use case.

@jthegedus
Copy link

jthegedus commented Jul 20, 2021

You can use either the JS SDK or the Admin SDK to do SSR with pros and cons(limitations) respectively. Which one to use is really based on your specific use case.

Precisely. I am finding that few people actually know what these specific pros/cons are, and people are assuming the result of using the JS SDK in SSR without addressing the limitations is the same as it is on the client, which is not the case.


Back to the OP issue, I observe this even when not using the SDK in SSR contexts in the SvelteKit dev server for [email protected].

@Feiyang1
Copy link
Member

This should have been fixed in 9.0.0-beta.7.

@jthegedus
Copy link

jthegedus commented Jul 22, 2021

9.0.0-beta.7 resolved the initial issue, thanks.

This will probably need to be raised in the Admin SDK repo as well, but I get a deprecation warning for the exports folder mapping - https://nodejs.org/api/deprecations.html#DEP0148

@Feiyang1
Copy link
Member

@jthegedus Is it only an issue on the Admin SDK? I just checked our package.json, and we don't use trailing '/'s anywhere. See https://unpkg.com/browse/[email protected]/package.json

@jthegedus
Copy link

jthegedus commented Jul 22, 2021

This is the full warning output:

(node:180461) [DEP0148] DeprecationWarning: Use of deprecated folder mapping "./" in the "exports" field module resolution of the package at /home/jthegedus/projects/test/site/node_modules/.pnpm/@[email protected]/node_modules/tslib/package.json.
Update this package.json to use a subpath pattern like "./*".
(Use `node --trace-deprecation ...` to show where the warning was created)
(node:180461) [DEP0148] DeprecationWarning: Use of deprecated folder mapping "./" in the "exports" field module resolution of the package at /home/jthegedus/projects/test/site/node_modules/.pnpm/@[email protected]/node_modules/tslib/package.json.
Update this package.json to use a subpath pattern like "./*".

Depedencies of dependencies it seems. Though a warning I did not receive on 9.0.0-beta.6.

@firebase firebase locked and limited conversation to collaborators Aug 22, 2021
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

No branches or pull requests

8 participants