Skip to content
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

Use esbuild lib instead of npx #7395

Merged
merged 9 commits into from
Aug 2, 2024

Conversation

chalosalvador
Copy link
Member

@chalosalvador chalosalvador commented Jun 27, 2024

Description

Fix for this #7250 and #6193 where esbuild execution was throwing an error saying "Command line too long" on Windows.

It tries to find an installed version of esbuild using npm which, if it is not found then it tries to install it. Once it is able to find a path to esbuild, it dynamically imports it and uses it to bundle the next.config file.

If the version of the installed esbuild doesn't match the required version, it will show a warning in the terminal.

Scenarios Tested

@leoortizz leoortizz requested a review from jamesdaniels June 27, 2024 17:44
@leoortizz
Copy link
Member

@jamesdaniels this would address #7250 and #6193, WDYT?

@jamesdaniels
Copy link
Member

We use to do this. The trouble here is that firebase-tools pins dependencies. So we were pinning to a version of esbuild that was not supported on customer's machines, be it node version or architecture.

Perhaps we could be smart and do some equivalent of "npm install" and "npm which"

src/frameworks/next/index.ts Outdated Show resolved Hide resolved
- Add `findEsbuildPath`: Helper function to find the path of esbuild using `npm which`

- Add `installEsbuild`: Helper function to install esbuild dynamically

- Add tests for `findEsbuildPath` and `installEsbuild`
@chalosalvador chalosalvador marked this pull request as ready for review July 11, 2024 16:30
@chalosalvador chalosalvador merged commit 115dfe5 into master Aug 2, 2024
41 checks passed
@chalosalvador chalosalvador deleted the chalosalvador/esbuild-command-line-too-long branch August 2, 2024 15:27
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