simondotm / nx-firebase

Firebase plugin for Nx Monorepos
https://www.npmjs.com/package/@simondotm/nx-firebase
MIT License
175 stars 31 forks source link

Firebase function V1 failed to deploy #158

Closed chrisDupuis closed 7 months ago

chrisDupuis commented 8 months ago

When I use a V1 mode function I have an error

i  functions: Loading and analyzing source code for codebase back-firebase-api-v0 to determine what to deploy
Serving at port 8855
shutdown requested via /__/quitquitquit
Error: Functions codebase could not be analyzed successfully. It may have a syntax or runtime error
Warning: run-commands command "firebase --config=firebase.back-firebase.json deploy --only functions:back-firebase-api-v0:helloWorldV0" exited with non-zero status code

The function

export const helloWorldV10 = functions.region("us-central1")
.https.onRequest((req, res) => {
    res.send(` I am a function.`);
  }
);

If I only use V2 function the déploiement is OK

i  functions: Loading and analyzing source code for codebase back-firebase-api-v0 to determine what to deploy
Serving at port 9000
shutdown requested via /__/quitquitquit
i  functions: Loaded environment variables from .env.
...
export const helloWorldV0 = onRequest((request, response) => {
  logger.info('Hello logs!', { structuredData: true });
  response.send('Hello from Firebase!');
});

with version "@simondotm/nx-firebase": "^2.1.2",

chrisDupuis commented 8 months ago

Does anyone have deploy V1 firebase function with this new version of plugin ?

simondotm commented 8 months ago

Hi @chrisDupuis - I still use v1 firebase projects in one of my other projects, and last time I deployed a few months ago it was ok. Will check when I get a minute, if in the meantime you are able to share the entire function main.ts that is failing that could be helpful.

chrisDupuis commented 8 months ago

We have create a test project ( before doing the migration). Here is the simpliest test project that failed.


import { initializeApp } from "firebase-admin/app";
initializeApp()

import { onRequest } from 'firebase-functions/v2/https';
import * as logger from 'firebase-functions/logger';
import functions from 'firebase-functions/v1';

export const helloWorld0_V2 = onRequest((request, response) => {
  logger.info('Hello logs!', { structuredData: true });
  response.send('Hello from Firebase!');
});

// en error
// En firebase function V1
exports.helloWorld_V1 = functions
.region("us-central1")
.https.onRequest(async (req, res) => {
  res.send('Hello from Firebase!');
});
ciriousjoker commented 8 months ago

@chrisDupuis What's the actual error message when you run something like this instead: npx firebase deploy --config firebase.yourproject.json --only functions --debug Debug logs are in firebase-debug.log

In my case, the underlying issue was an import statement: I used import sub from "date-fns/sub"; instead of import { sub } from "date-fns"; and apparently firebase can't handle that.

I suspect the issue are your logger & functions imports. Try rewriting those and see if that helps.

If that was indeed the issue though, I feel like this is something that should be solved by the build pipeline instead. Rewriting many imports isn't really feasable and it's only a matter of time before some dev accidentally accepts the wrong import and breaks deployment with very hard to find error logs.

logs to compare against > [info] i functions: Loading and analyzing source code for codebase backend to determine what to deploy > [debug] [2023-10-21T22:28:13.057Z] Could not find functions.yaml. Must use http discovery > [debug] [2023-10-21T22:28:13.061Z] Found firebase-functions binary at '/projectpath/node_modules/.bin/firebase-functions' > [info] Serving at port 8160 > > [debug] [2023-10-21T22:28:13.240Z] Got response code 400; body Failed to generate manifest from function source: Error [ERR_UNSUPPORTED_DIR_IMPORT]: Directory import '/projectpath/node_modules/date-fns/sub' is not supported resolving ES modules imported from /projectpath/dist/apps/backend/index.js > Did you mean to import date-fns/sub/index.js? > [info] shutdown requested via /__/quitquitquit
ciriousjoker commented 8 months ago

I did some more digging:

Switching the format in project.json from "esm" to "cjs" makes it so everything deploys, but it breaks during runtime in a specific case for us:

import * as admin from "firebase-admin";

...

// here we check if Firebase is already initialized to avoid calling initializeApp() twice
const isInitialized = admin.apps.some((app) => app?.name === "[DEFAULT]");

What this code does isn't important, the important part is that the imported admin variable only contains apps during editing. Then, at runtime, admin actually contains default, which then contains apps. Obviously this breaks and can't be used in production.

I've tried to make esbuild work like this so far, but nothing seems to work:

ciriousjoker commented 8 months ago

So I've managed to finally deploy a sample cloud function, here's what lead to that:

Not sure if cjs improves any of this. I think there's one "path" I could test there which might work, since the error message that I gave up on was the same error that was fixed by updating the klaviyo-api version. I would assume that this would be fixed and since that was the last error hurdle to get over, the deployment would have probably worked. That being said, I spend 10+ hours on getting deployment to work and at this point idc to test this anymore since esm is preferred anyway.

simondotm commented 8 months ago

Interesting findings, thanks.

Yes firebase exports in the later SDKs are now modular - see here

I did add some notes on use of es6 modules to the docs, but this topic makes me think there are corner cases - especially if using package dependencies that do not have an esm export, only cjs.

Note also that the package.json having type: "module" is another esm variable that affects how node projects are built/consumed.

I found that migrating to esm was a bit of 'a process' in my own projects, but I believe it is inevitable that cjs modules are on track to become legacy now, so I felt it was worth putting in the grind.

simondotm commented 7 months ago

@chrisDupuis - did you make any progress with this on your side?

brendanowens commented 7 months ago

I believe I am having the same (or a very similar issue). I can spin it into its own issue if needed.

Here is a minimal reproduction of the issue:

With the following main.ts:

import { initializeApp } from 'firebase-admin/app';
import { getFirestore } from 'firebase-admin/firestore';
export const app = initializeApp();
export const db = getFirestore();
import { https } from 'firebase-functions';

export const helloWorld_V1 = https.onRequest(async (req, res) => {
  res.send('Hello from Firebase!');
});

I receive the following error when running nx server [firebase-app-name]:

Error: Dynamic require of "fs" is not supported
    at file:///Users/brendanowens/projects/layer/dist/apps/layer-functions/main.js:12:9
    at node_modules/firebase-admin/lib/app/credential-internal.js (file:///Users/brendanowens/projects/layer/dist/apps/layer-functions/main.js:32642:14)
    at __require2 (file:///Users/brendanowens/projects/layer/dist/apps/layer-functions/main.js:18:51)
    at node_modules/firebase-admin/lib/utils/index.js (file:///Users/brendanowens/projects/layer/dist/apps/layer-functions/main.js:33319:33)
    at __require2 (file:///Users/brendanowens/projects/layer/dist/apps/layer-functions/main.js:18:51)
    at node_modules/firebase-admin/lib/app/index.js (file:///Users/brendanowens/projects/layer/dist/apps/layer-functions/main.js:33787:19)
    at __require2 (file:///Users/brendanowens/projects/layer/dist/apps/layer-functions/main.js:18:51)
    at file:///Users/brendanowens/projects/layer/dist/apps/layer-functions/main.js:190526:26
    at ModuleJob.run (node:internal/modules/esm/module_job:193:25)
    at async Promise.all (index 0)

Note that if I switch the import from firebase-functions to be the first import (rather than the import from firebase-admin), then I receieve a similar error of Dynamic require of "util" is not supported bubbling up from the firebase-functions library.

Relevant versions: "firebase-admin": "11.11.0", "firebase-functions": "4.5.0", "@simondotm/nx-firebase": "2.1.2", "firebase-tools": "12.8.0", All @nx dependencies are on 16.10.0

My build config in my project.json ``` "build": { "executor": "@nx/esbuild:esbuild", "outputs": ["{options.outputPath}"], "options": { "outputPath": "dist/apps/layer-functions", "main": "apps/layer-functions/src/main.ts", "tsConfig": "apps/layer-functions/tsconfig.app.json", "assets": [ { "input": "apps/layer-functions/src/assets", "glob": "**/*", "output": "assets" }, { "glob": "**/*", "input": "apps/layer-firebase/environment", "output": "." }, { "input": "apps/layer-functions/ghostscript", "glob": "**/*", "output": "ghostscript" } ], "generatePackageJson": true, "platform": "node", "bundle": true, "thirdParty": false, "dependenciesFieldType": "dependencies", "target": "node16", "format": ["esm"], "esbuildOptions": { "logLevel": "info", "external": ["canvas", "sharp"] } } } ```
My tsconfig.app.json ``` { "extends": "./tsconfig.json", "compilerOptions": { "module": "commonjs", "noImplicitReturns": true, "outDir": "lib", "sourceMap": true, "target": "ES2021", "lib": ["ES2021", "dom"], "moduleResolution": "node", "allowSyntheticDefaultImports": true, "esModuleInterop": true }, "exclude": ["**/*.spec.ts", "jest.config.ts"], "include": ["src/**/*.ts", "src/**/*.tsx"] } ```
My functions package.json ``` { "name": "layer-functions", "description": "Firebase Function, auto generated by @simondotm/nx-firebase", "scripts": { "test": "nx test layer-functions", "lint": "nx lint layer-functions", "build": "nx build layer-functions", "deploy": "nx deploy layer-functions" }, "type": "module", "engines": { "node": "16" }, "dependencies": {}, "devDependencies": {}, "private": true } ```

Some notes:

simondotm commented 7 months ago

Thanks @brendanowens this is helpful. I'm wondering if import { https } from 'firebase-functions'; is the problem, maybe that is importing the cjs version of firebase functions or something.

Could you try import { https } from 'firebase-functions/v1'; instead? That might fix it.

simondotm commented 7 months ago

I believe that firebase team are generally recommending against using the old style imports like:

import * as admin from "firebase-admin";
admin.initializeApp();

..etc.

to the new modular approach where you'd import specifically the dependencies you need:

import { initializeApp } from "firebase-admin/app";
initializeApp()

This is the recommended import recipe regardless of whether output will be built as esm or cjs.

brendanowens commented 7 months ago

Thanks @simondotm for the suggestion, but the issue still persists with the following imports:

import { https } from 'firebase-functions/v1';

import { initializeApp } from 'firebase-admin/app';
import { getFirestore } from 'firebase-admin/firestore';
brendanowens commented 7 months ago

Adding the following banner to my project.json esbuildOptions seems to resolve the error, but I am not exactly sure why it is necessary and what the ramifications of doing it will be. This was taken from this comment. Any thoughts on why this is needed with esbuild @simondotm?

"esbuildOptions": {
  ...
  "banner": {
    "js": "const require = (await import('node:module')).createRequire(import.meta.url);const __filename = (await import('node:url')).fileURLToPath(import.meta.url);const __dirname = (await import('node:path')).dirname(__filename);"
  }
}
ciriousjoker commented 7 months ago

@brendanowens esbuild uses esm by default and esm doesnt support node's global __dirname variables. The code snippet essentially patches that.

To debug, remove imports until compilation works and see if you can change the broken imports to something else, import them in a different way or avoid them entirely.