Skip to content

npm list fallback, increase timeout to 60s #8702

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

Merged
merged 4 commits into from
Jun 4, 2025
Merged

Conversation

leoortizz
Copy link
Member

@leoortizz leoortizz commented Jun 3, 2025

Description

This PR aims to fix the issue reported in #8689. The npm command timeout was increased to 60s due to how slow it runs in non-npm projects, notably with pnpm as reported in the issue.

Additionally, a fallback was added in case the timeout is reached, where the dependency version is read directly from its package.json within node_modules.

Scenarios Tested

deploy Next.js project with pnpm

Sample Commands

firebase deploy

@alexastrum alexastrum marked this pull request as ready for review June 3, 2025 21:06
Copy link
Contributor

@alexastrum alexastrum left a comment

Choose a reason for hiding this comment

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

pls fix tests.

@github-project-automation github-project-automation bot moved this to Changes Requested [PR] in [Cloud] Extensions + Functions Jun 3, 2025
} catch (e) {
// fallback to reading the version directly from package.json if npm list times out
const packageJson = readJsonSync(join(cwd, name, "package.json"), { throws: false });
return packageJson?.version ? { version: packageJson.version } : undefined;
Copy link
Member Author

Choose a reason for hiding this comment

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

if we choose to use this, we might want to mimic npm list json structure here. Currently all places calling this function are only using the version, so I only implemented the version for now

@leoortizz
Copy link
Member Author

leoortizz commented Jun 3, 2025

pls fix tests.

@alexastrum FWICT the failing tests are unrelated to these changes.

  • vscode_integration is also failing in the master branch and all other PRs. It has been like that for a while.
  • test:extensions-emulator passed after a retry.

@github-project-automation github-project-automation bot moved this from Changes Requested [PR] to Approved [PR] in [Cloud] Extensions + Functions Jun 3, 2025
@leoortizz leoortizz enabled auto-merge (squash) June 4, 2025 13:53
@leoortizz leoortizz self-assigned this Jun 4, 2025
@leoortizz leoortizz merged commit b5c6238 into master Jun 4, 2025
48 of 50 checks passed
@leoortizz leoortizz deleted the leoortizz_8689 branch June 4, 2025 14:04
@github-project-automation github-project-automation bot moved this from Approved [PR] to Done in [Cloud] Extensions + Functions Jun 4, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants