firebase / firebase-tools

The Firebase Command Line Tools
MIT License
3.98k stars 918 forks source link

Functions emulator should only ignore what is in `functions.ignore` #2131

Closed samtstern closed 1 week ago

samtstern commented 4 years ago

[REQUIRED] Environment info

firebase-tools: all

Platform: all

[REQUIRED] Test case

Right now we have the following file watcher in the Functions emulator: https://github.com/firebase/firebase-tools/blob/bd6a23be0ed9a7b06aa178aa66c94e6cb6ce8045/src/emulator/functionsEmulator.ts#L238

It ignores certain files based on best practices, however in firebase.json the developer can customize functions.ignore, we should match that exactly with our file watcher.

The main use case for this is when developers want to use local node modules, we are excluding them by matching all node_modules

[REQUIRED] Steps to reproduce

N/A

[REQUIRED] Expected behavior

See above

[REQUIRED] Actual behavior

See above

TacB0sS commented 4 years ago

I am under the impression that function.ignore is also responsible for determining which part of the directory tree and files would not be deployed to the cloud function!

On the one hand serving locally I don't want to ignore the node_modules cause I work on the project and the library and would like the function to listen on changes there while on the other hand I do not want to deploy the node_modules folder to the cloud function.

I believe this makes the function.ignore have a dual functionality that is potentially conflicting therefore imo should not be coupled.

Perhaps another entry in the firebase.json file would be a better fit?


I have also ran a simple test and can verify that if I include the node_modules in the ignore it will not deploy that folder whereas if I omit it, it will deploy with the node_modules folder which substantially increase the deployment size...

firebase-tools version: 8.1.0

samtstern commented 4 years ago

@TacB0sS thanks for the feedback. As you said, functions.ignore controls what is actually uploaded to Cloud Functions. We want the local development environment to help you deploy confidently, so ignoring the same files means you will have almost the same experience locally and in prod. If you're relying on something that is in node_modules locally your functions will break in production!

Does that makes sense or am I misunderstanding your concern?

TacB0sS commented 4 years ago

This is our firebase.json file:

{
  "functions": {
    "source": "app-backend",
    "ignore": [
      "src",
      ".config",
      "dist-test",
      "deploy.js",
      "node_modules"
    ],
    "predeploy": [
      "npm --prefix ./app-backend run build"
    ]
  },
  "hosting": {
    "public": "app-frontend/dist/dev"
    "rewrites": [
      {
        "source": "**",
        "destination": "/index.html"
      }
    ]
  }
}

as we are developing both frontend and backend in the same workspace we have them in separate folders app-backend and app-frontend respectively.

from what I understand the assumption is that node_module would always be in the root folder of the project, and we have it in the "source" as defined in the firebase.json.

as we have different firebase.json per environment including local-dev, I can simply remove the node_module from the ignore in the dev firebase.json

mikehardy commented 1 year ago

I'm interested in seeing this resolved more because if you for instance test your functions locally as an API via jest+supertest with coverage enabled etc, you get a coverage directory, and the functions emulator reloads every test run

That's pretty wasteful when running jest in watch mode and developing quickly, it also leads to some races where the jest tries to run a test that hits the API but the API is reloading and it hangs etc.

Is this on the radar for anyone? Just curious

an embarrassing brutality of a hack, for patch-package


diff --git a/node_modules/firebase-tools/lib/emulator/functionsEmulator.js b/node_modules/firebase-tools/lib/emulator/functionsEmulator.js
index aeb8961..5623970 100644
--- a/node_modules/firebase-tools/lib/emulator/functionsEmulator.js
+++ b/node_modules/firebase-tools/lib/emulator/functionsEmulator.js
@@ -214,6 +214,10 @@ class FunctionsEmulator {
                     /.+?[\\\/]node_modules[\\\/].+?/,
                     /(^|[\/\\])\../,
                     /.+\.log/,
+                    /__tests__/,
+                    /__mocks__/,
+                    /firebase-export-/,
+                    /coverage/,
                 ],
                 persistent: true,
             });
sceee commented 1 year ago

This issue also occurs when trying to export data while the emulator is running: So having firebase emulators:start --import=./testdata/emulatordata to be running in one terminal and executing firebase emulators:export ./testdata/emulatordata in a second terminal causes the functions to reload (during the export) and causes the export to fail with the following log:

Error: Export request failed, see emulator logs for more information.

In the emulator logs, it shows something like:

!  emulators: Export failed: EPERM: operation not permitted, rename 'firebase-export-123456789' -> 'C:\my\path\to\project\testdata\emulatordata'

This used to be working in the past.

dikatok commented 3 weeks ago

@samtstern is this open for contribution? I happen to encounter this issue where I have 1 codebase for 2 processes (node and firebase functions) and the functions emulator keeps reloading whenever the node process modifies unrelated files. And modifying functions.ignore should have preventing both deployment and emulator from uploading the ignored files (which in the emulator case, should also be ignored during watch).

joehan commented 3 weeks ago

@dikatok This is definitely open for contribution! Feel free to open a PR, and we'll find an appropriate reviewer for it.

dikatok commented 1 week ago

@joehan ok, I'll see what I can do this weekend