simondotm / nx-firebase

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

Functions that use local libraries do not deploy with Firebase Node 12 engine #24

Closed simondotm closed 1 year ago

simondotm commented 3 years ago

Firebase Node 12 support uses npm ci rather than npm install - which requires a package-lock.json which the plugin does not generate. This could be a problem. Need to check. https://cloud.google.com/functions/docs/migrating/nodejs-runtimes#nodejs-12-changes

simondotm commented 3 years ago

Hmm I just deployed a bunch of functions to a node 12 environment with no package-lock.json and no issues. Docs maybe incorrectly suggesting that package-lock.json is required.

simondotm commented 3 years ago

I'm pretty confident that a package-lock.json file is not required from node12+. npm ci will use it if it exists, but works fine if it doesn't. Which is good news since we do not want to have to create a package-lock.json file just to deploy.

simondotm commented 3 years ago

Nope, reopening as this is a real issue.

For functions that use local libraries, deploying to nodeengine 12 onwards will error because there isn't a package-lock.json file. image

I believe my earlier test worked ok because it didn't import shared libs.

simondotm commented 3 years ago

Need to figure out how to sort this. Maybe synthesize a package-lock.json file? Ideally a simpler solution exists.

Schmale97 commented 3 years ago

I was able to deploy a cloud function to a project using Node 12 where a shared lib is imported. There is no package-lock.json in my app directory nor my library directory. What conditions is this error happening for you?

simondotm commented 3 years ago

@Schmale97 Thanks for letting me know you've managed to do this without issue, that's extremely useful to know.

I have about 12 libs in my project and a recent test deploy with node 12 just blew up as per the image above. I am a bit suspicious though that it might be some other subtle issue. I've reverted to node 10 for now and will take a closer look next week if I get a bit of spare time.

Schmale97 commented 3 years ago

@simondotm no worries, my codebase is very simple at the moment, I have one app 'functions-app' which has the 'helloWorld' cloud function which this package generates. And one lib 'my-lib' the boilerplate lib generated from 'nx g @nrwl/node:lib my-lib --buildable'. I have then imported that lib and added the string from 'my-lib' to the logging in the 'helloWorld' function.

not sure if that is useful information - let me know if you need any info when you look at it

simondotm commented 3 years ago

Thanks. I'm wondering if it might be because some of my local libs depend on other local libs which might be opening up a can of peer deps worms and I might have to process the package.json for every library being deployed to use local library references in this scenario.

simondotm commented 3 years ago

In fact just saying that out loud has convinced me that is what the problem likely is.

Schmale97 commented 3 years ago

not using nx - there is an issue currently with firebase (https://github.com/firebase/firebase-tools/issues/968) to do with the deploy step failing when using local depencies with npm - there are some workarounds at the bottom of the thread - that may not be relevant here though

simondotm commented 3 years ago

Yes I've stumbled across that epic thread before! I documented my own experience and solution in the readme

_The reason why we make an additional copy of dependent libraries to the nodemodules folder is because the Firebase CLI runs some checks (and partially processes the primary functions entry point node script) prior to deployment to ensure everything is in order.

_There's no need to do a full npm install here, we only have to ensure local libraries exist inside nodemodules at deployment time.

Schmale97 commented 3 years ago

Not sure where you have got to with this, but I just encountered an issue with deploying when I created a 2nd lib 'their-lib' which 'my-lib' depended on. Removing the dependency and the lib stopped the error from occurring.

simondotm commented 3 years ago

Ah ok, that pretty much confirms my suspicions then. I'll need to update the plugin to modify the package.json for each dependent lib that depends on another local lib. Give me a couple of days to see if I can get this sorted.

simondotm commented 3 years ago

FWIW I'm not certain if there's any compelling reason to use node12 over node10 with firebase functions, so changing your nodeengine to 10 is likely to be an acceptable workaround for now

Schmale97 commented 3 years ago

Yeah can confirm that changing to node 10 stops the deploy error for me, thanks :)

cogoo commented 3 years ago

Thanks @simondotm for this awesome library. I ran into the same issue with Node12. Downgrading to Node10 fixes the issue and as your mentioned don't see any issues with using the Node10 issue

simondotm commented 3 years ago

Folks, I'm looking into the node12 support atm, but it might take a couple of weeks to complete.

BookmanAG commented 2 years ago

So the current workaround only involves deploying with Node engine 10 rather than 12? Because I just faced the same issue today and now wondering how to solve it D:

EDIT Using Node v10 solves the issue however be aware that today a new version of firebase adin SDK got released which dropped support for Node v10.

More info: https://github.com/firebase/firebase-admin-node/releases/tag/v10.0.0 cc @simondotm

BookmanAG commented 2 years ago

Hi all users and @simondotm

Since today for some unknown reason the Node 10 deployment of CF with dependant libs (all buildables) is failing with the exact same error as if I were deploying with Node 12. Has anyone faced this issue so far?

simondotm commented 1 year ago

I think given the age of this issue and that Node 16 is probably the minimum target deployment version folks should be using now, we should consider Node 10 and Node 12 as no longer supported.