Skip to content

Conversation

@webbertakken
Copy link
Member

@webbertakken webbertakken commented Oct 8, 2025

Changes

Checklist

  • Read the contribution guide and accept the code of conduct
  • Readme (updated or not needed)
  • Tests (added, updated or not needed)

Summary by CodeRabbit

  • Chores

    • Updated Cloud Functions runtime to Node.js 24 and bumped functions' Node engine.
    • Standardized tooling on Yarn 4.10.3 with node-modules linking and added package manager/Volta config.
    • Removed GraphQL dependency from Functions.
    • Expanded ignore rules to exclude Yarn artifacts.
  • CI

    • CI workflows now install dependencies explicitly and derive node version from package metadata.

@coderabbitai
Copy link
Contributor

coderabbitai bot commented Oct 8, 2025

Warning

Rate limit exceeded

@webbertakken has exceeded the limit for the number of commits or files that can be reviewed per hour. Please wait 13 minutes and 9 seconds before requesting another review.

⌛ How to resolve this issue?

After the wait time has elapsed, a review can be triggered using the @coderabbitai review command as a PR comment. Alternatively, push new commits to this PR.

We recommend that you space out your commits to avoid hitting the rate limit.

🚦 How do rate limits work?

CodeRabbit enforces hourly rate limits for each developer per organization.

Our paid plans have higher rate limits than the trial, open-source and free plans. In all cases, we re-allow further reviews after a brief timeout.

Please see our FAQ for further information.

📥 Commits

Reviewing files that changed from the base of the PR and between 65b5152 and 3ac1f33.

⛔ Files ignored due to path filters (2)
  • functions/yarn.lock is excluded by !**/yarn.lock, !**/*.lock
  • yarn.lock is excluded by !**/yarn.lock, !**/*.lock
📒 Files selected for processing (4)
  • .github/workflows/main.yml (2 hunks)
  • README.md (1 hunks)
  • firebase.json (1 hunks)
  • package.json (2 hunks)

Walkthrough

Repository configuration updates: add Yarn ignore entries, set Yarn nodeLinker to node-modules, add/standardize Yarn v4 packageManager metadata and Volta fields, bump Node engine metadata to Node 22 in functions and root tooling, remove GraphQL and bump unity-changeset in functions, set Firebase Functions runtime to nodejs24, and update GitHub Actions to install dependencies via Yarn before build/test/deploy.

Changes

Cohort / File(s) Summary
Git ignore updates
/.gitignore
Add /.yarn/ and /functions/.yarn/ to ignore Yarn cache directories.
Yarn configuration
/.yarnrc.yml
Set nodeLinker: node-modules.
Firebase Functions runtime
/firebase.json
Add functions.runtime: "nodejs24".
Functions package & deps
/functions/package.json
Bump engines.node to Node 22; remove graphql dependency; upgrade unity-changeset to ^3.0.1; add packageManager: "[email protected]" and remove Volta block in favor of packageManager metadata.
Root package metadata
/package.json
Add volta with Node 22.20.0 and Yarn 4.10.3; add packageManager: "[email protected]".
CI workflow changes
.github/workflows/main.yml
Replace terse setup with explicit Checkout / Install package manager (corepack) / Setup Node using node-version-file: package.json; add yarn install --immutable and yarn --cwd ./functions install --immutable before lint/test/build/deploy steps across jobs.

Sequence Diagram(s)

sequenceDiagram
  autonumber
  participant Dev as Developer / PR
  participant GH as GitHub Actions
  participant Corepack as Corepack (yarn)
  participant Node as Node toolchain (from package.json)
  participant YarnRoot as Yarn (root)
  participant YarnFns as Yarn (functions)
  participant Firebase as Firebase Deploy

  Dev->>GH: push PR
  GH->>Corepack: enable/install corepack
  GH->>Node: setup Node (node-version-file: package.json)
  GH->>YarnRoot: run `yarn install --immutable`
  GH->>YarnFns: run `yarn --cwd ./functions install --immutable`
  alt build/test
    GH->>GH: run lint, tests, build
  end
  alt deploy
    GH->>Firebase: predeploy & deploy functions (runtime: nodejs24)
  end
Loading

Estimated code review effort

🎯 2 (Simple) | ⏱️ ~10 minutes

Possibly related PRs

Suggested reviewers

  • GabLeRoux

Poem

Hoppity-hop, I tidy the lair,
Yarn hums in folders, calm and fair,
Node set to twenty-two, then twenty-four in deploy,
Old GraphQL hopped off—no ploy.
A rabbit's small cheer: clean repo, more joy. 🐇✨

Pre-merge checks and finishing touches

❌ Failed checks (1 warning)
Check name Status Explanation Resolution
Description Check ⚠️ Warning The pull request description includes the template structure but the Changes section remains a placeholder (“…”), providing no information on the actual modifications made, which prevents reviewers from understanding the scope and intent of the updates. Please replace the placeholder in the Changes section with a concise summary of the specific updates (for example, the Yarn and Node version bumps, unity-changeset upgrade, gitignore additions, workflow adjustments, and Firebase runtime change) and ensure any other template fields are appropriately completed or marked as not needed.
✅ Passed checks (2 passed)
Check name Status Explanation
Title Check ✅ Passed The title “feat: bump toolset and unity-changeset” succinctly captures the primary changeset of updating the project’s toolset configurations and upgrading the unity-changeset dependency, aligning with conventional commit style and clearly reflecting the PR’s main focus.
Docstring Coverage ✅ Passed No functions found in the changes. Docstring coverage check skipped.

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

Comment @coderabbitai help to get the list of available commands and usage tips.

@GabLeRoux
Copy link
Member

Looks good to me, let's merge when the pipeline is green 🚀

Copy link
Contributor

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 2

Caution

Some comments are outside the diff and can’t be posted inline due to platform limitations.

⚠️ Outside diff range comments (1)
functions/package.json (1)

15-50: Pinned Node 24 requirement breaks Firebase deployment.

Because Cloud Functions only offers Node runtimes up to 22 today, setting both the engines.node field and Volta/tooling metadata to Node 24 makes yarn install fail during deploy and contradicts the platform runtime. Please revert to Node 20 (or another supported version) across these fields until Firebase enables Node 24.

-    "node": "24"
+    "node": "20"
@@
-  "volta": {
-    "node": "24.9.0",
-    "yarn": "4.10.3"
-  },
-  "packageManager": "[email protected]"
+  "volta": {
+    "node": "20.17.0",
+    "yarn": "4.10.3"
+  },
+  "packageManager": "[email protected]"

(Adjust the exact Node 20.x patch as appropriate.)

📜 Review details

Configuration used: CodeRabbit UI

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 9fd589e and 7edd0e3.

⛔ Files ignored due to path filters (2)
  • functions/yarn.lock is excluded by !**/yarn.lock, !**/*.lock
  • yarn.lock is excluded by !**/yarn.lock, !**/*.lock
📒 Files selected for processing (5)
  • .gitignore (1 hunks)
  • .yarnrc.yml (1 hunks)
  • firebase.json (1 hunks)
  • functions/package.json (3 hunks)
  • package.json (1 hunks)

Comment on lines 34 to 38
"volta": {
"node": "24.9.0",
"yarn": "4.10.3"
},
"packageManager": "[email protected]"
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🟠 Major

Keep repo Volta/Node settings aligned with supported runtime.

Since the deployment target cannot run Node 24 yet, this new Volta block will steer contributors toward an unsupported version. Please match the Cloud Functions runtime (currently Node 20) so local tooling mirrors production.

-  "volta": {
-    "node": "24.9.0",
-    "yarn": "4.10.3"
-  },
+  "volta": {
+    "node": "20.17.0",
+    "yarn": "4.10.3"
+  },
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
"volta": {
"node": "24.9.0",
"yarn": "4.10.3"
},
"packageManager": "[email protected]"
"volta": {
"node": "20.17.0",
"yarn": "4.10.3"
},
"packageManager": "[email protected]"
🤖 Prompt for AI Agents
In package.json around lines 34 to 38, the Volta node version is set to 24.9.0
which diverges from the deployed Cloud Functions runtime; update the
"volta.node" value to the Node 20 runtime used in production (e.g., "20" or the
exact Cloud Functions patch version) so local tooling matches the supported
environment, leaving the yarn entry unchanged.

Copy link
Contributor

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 1

📜 Review details

Configuration used: CodeRabbit UI

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 7edd0e3 and 956f41b.

📒 Files selected for processing (1)
  • .github/workflows/main.yml (3 hunks)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
  • GitHub Check: Test Deploy

@GabLeRoux
Copy link
Member

@coderabbitai

`Test Deploy (pull_request)` job is failing

/usr/bin/docker run --name w9jdsfirebaseactionv13250_0c0230 --label eaf1c5 --workdir /github/workspace --rm -e "GCP_SA_KEY" -e "INPUT_ARGS" -e "HOME" -e "GITHUB_JOB" -e "GITHUB_REF" -e "GITHUB_SHA" -e "GITHUB_REPOSITORY" -e "GITHUB_REPOSITORY_OWNER" -e "GITHUB_REPOSITORY_OWNER_ID" -e "GITHUB_RUN_ID" -e "GITHUB_RUN_NUMBER" -e "GITHUB_RETENTION_DAYS" -e "GITHUB_RUN_ATTEMPT" -e "GITHUB_ACTOR_ID" -e "GITHUB_ACTOR" -e "GITHUB_WORKFLOW" -e "GITHUB_HEAD_REF" -e "GITHUB_BASE_REF" -e "GITHUB_EVENT_NAME" -e "GITHUB_SERVER_URL" -e "GITHUB_API_URL" -e "GITHUB_GRAPHQL_URL" -e "GITHUB_REF_NAME" -e "GITHUB_REF_PROTECTED" -e "GITHUB_REF_TYPE" -e "GITHUB_WORKFLOW_REF" -e "GITHUB_WORKFLOW_SHA" -e "GITHUB_REPOSITORY_ID" -e "GITHUB_TRIGGERING_ACTOR" -e "GITHUB_WORKSPACE" -e "GITHUB_ACTION" -e "GITHUB_EVENT_PATH" -e "GITHUB_ACTION_REPOSITORY" -e "GITHUB_ACTION_REF" -e "GITHUB_PATH" -e "GITHUB_ENV" -e "GITHUB_STEP_SUMMARY" -e "GITHUB_STATE" -e "GITHUB_OUTPUT" -e "RUNNER_OS" -e "RUNNER_ARCH" -e "RUNNER_NAME" -e "RUNNER_ENVIRONMENT" -e "RUNNER_TOOL_CACHE" -e "RUNNER_TEMP" -e "RUNNER_WORKSPACE" -e "ACTIONS_RUNTIME_URL" -e "ACTIONS_RUNTIME_TOKEN" -e "ACTIONS_CACHE_URL" -e "ACTIONS_RESULTS_URL" -e GITHUB_ACTIONS=true -e CI=true -v "/var/run/docker.sock":"/var/run/docker.sock" -v "/home/runner/work/_temp/_github_home":"/github/home" -v "/home/runner/work/_temp/_github_workflow":"/github/workflow" -v "/home/runner/work/_temp/_runner_file_commands":"/github/file_commands" -v "/home/runner/work/versioning-backend/versioning-backend":"/github/workspace" w9jds/firebase-action:v13.25.0 deploy --only functions:testFunction
Storing the decoded GCP_SA_KEY in /opt/gcp_key.json
Exporting GOOGLE_APPLICATION_CREDENTIALS=/opt/gcp_key.json

=== Deploying to 'unity-ci-versions'...

i  deploying functions
Running command: yarn --cwd "$RESOURCE_DIR" lint
error This project's package.json defines "packageManager": "[email protected]". However the current global version of Yarn is 1.22.22.

Presence of the "packageManager" field indicates that the project is meant to be used with Corepack, a tool included by default with all official Node.js distributions starting from 16.9 and 14.19.
Corepack must currently be enabled by running corepack enable in your terminal. For more information, check out https://yarnpkg.com/corepack.

Error: functions predeploy error: Command terminated with non-zero exit code 1

What would you do?

@coderabbitai
Copy link
Contributor

coderabbitai bot commented Oct 8, 2025

Tip

For best results, initiate chat on the files or code changes.

🧩 Analysis chain

🏁 Script executed:

#!/bin/bash
# Check the workflow file to see the current setup
cat .github/workflows/main.yml

Length of output: 3552


🏁 Script executed:

#!/bin/bash
# Check firebase.json to see the predeploy commands
cat firebase.json

Length of output: 716


🏁 Script executed:

#!/bin/bash
# Check if there are any other relevant configuration files
cat functions/package.json | jq '{engines, packageManager, volta}'

Length of output: 136


@GabLeRoux, I can see the issue! The problem is that the w9jds/[email protected] runs in its own Docker container which has Yarn 1.22.22 installed, but your project now requires Yarn 4.10.3 (specified in the packageManager field).

Root Cause

When Firebase CLI tries to run the predeploy commands defined in firebase.json, it uses the container's global Yarn 1.22.22, which conflicts with the packageManager: "[email protected]" requirement.

Recommended Solution

Remove the predeploy commands from firebase.json since your workflow already runs lint, test, and build steps successfully before the Firebase deployment:

{
  "firestore": {
    "rules": "firestore.rules",
    "indexes": "firestore.indexes.json"
  },
  "functions": {
    "runtime": "nodejs24"
  },
  "hosting": {
    "public": "public",
    "ignore": [
      "firebase.json",
      "**/.*",
      "**/node_modules/**"
    ]
  },
  "storage": {
    "rules": "storage.rules"
  },
  "emulators": {
    "functions": {
      "port": 5001
    },
    "firestore": {
      "port": 8080
    },
    "hosting": {
      "port": 5002
    },
    "pubsub": {
      "port": 8085
    },
    "ui": {
      "enabled": true
    }
  }
}

This eliminates the redundant predeploy steps that are already handled by your CI pipeline.

Why This Works

Looking at your workflow, you already have proper gates in place:

  1. ✅ Lint runs in the test job
  2. ✅ Tests run in the test job
  3. ✅ Build runs in the build job
  4. testDeploy job depends on both test and build completing

So the predeploy hooks are unnecessary duplication and only cause issues with the Yarn version mismatch in the Firebase Action container.

🐰

@GabLeRoux
Copy link
Member

Tip

For best results, initiate chat on the files or code changes.

🧩 Analysis chain
🏁 Script executed:

#!/bin/bash
# Check the workflow file to see the current setup
cat .github/workflows/main.yml

Length of output: 3552

🏁 Script executed:

#!/bin/bash
# Check firebase.json to see the predeploy commands
cat firebase.json

Length of output: 716

🏁 Script executed:

#!/bin/bash
# Check if there are any other relevant configuration files
cat functions/package.json | jq '{engines, packageManager, volta}'

Length of output: 136

@GabLeRoux, I can see the issue! The problem is that the w9jds/[email protected] runs in its own Docker container which has Yarn 1.22.22 installed, but your project now requires Yarn 4.10.3 (specified in the packageManager field).

Root Cause

When Firebase CLI tries to run the predeploy commands defined in firebase.json, it uses the container's global Yarn 1.22.22, which conflicts with the packageManager: "[email protected]" requirement.

Recommended Solution

Remove the predeploy commands from firebase.json since your workflow already runs lint, test, and build steps successfully before the Firebase deployment:

{
  "firestore": {
    "rules": "firestore.rules",
    "indexes": "firestore.indexes.json"
  },
  "functions": {
    "runtime": "nodejs24"
  },
  "hosting": {
    "public": "public",
    "ignore": [
      "firebase.json",
      "**/.*",
      "**/node_modules/**"
    ]
  },
  "storage": {
    "rules": "storage.rules"
  },
  "emulators": {
    "functions": {
      "port": 5001
    },
    "firestore": {
      "port": 8080
    },
    "hosting": {
      "port": 5002
    },
    "pubsub": {
      "port": 8085
    },
    "ui": {
      "enabled": true
    }
  }
}

This eliminates the redundant predeploy steps that are already handled by your CI pipeline.

Why This Works

Looking at your workflow, you already have proper gates in place:

  1. ✅ Lint runs in the test job
  2. ✅ Tests run in the test job
  3. ✅ Build runs in the build job
  4. testDeploy job depends on both test and build completing

So the predeploy hooks are unnecessary duplication and only cause issues with the Yarn version mismatch in the Firebase Action container.

🐰

For reference, this was not the fix. There was a runtime value in firebase.json that was kept to node24. But it's true that firebase-action runs its own docker container with its own versions. :)

@webbertakken
Copy link
Member Author

We should probably remove this hack after w9jds/firebase-action#256 is resolved

@webbertakken webbertakken merged commit 0b3d7c8 into main Oct 8, 2025
6 checks passed
@webbertakken webbertakken deleted the bump-toolset branch October 8, 2025 21:24
@coderabbitai coderabbitai bot mentioned this pull request Oct 8, 2025
3 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants