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

moves esbuild to an optional dependency; requires node 16 for frameworks #5052

Merged
merged 2 commits into from
Sep 30, 2022

Conversation

bkendall
Copy link
Contributor

Description

esbuild as a dependency causes issues with npm@6 since it will fail installs for optional dependencies where engines don't match (from npm-shrinkwrap). Moving esbuild to optionalDependencies allows node 14 installs (which default to npm@6) to continue working.

firebase-frameworks has a requirement of node >= 16.0.0 and npm will complain if its codepaths are used on lower versions of node. Enforcing a requirement of node 16 or higher in the awareness codepaths will prevent issues from arising there and should give us confidence that npm is >=8, where the optional dependencies issue above won't happen either (and therefore esbuild should have been installed with firebase-tools).

But, as a precaution, we must move and check the import for esbuild later in the runtime so that we don't try to load a dependency that may have failed to install (notably on node 14).

Scenarios Tested

I've been testing with a local nextjs app on nodes 14 and 16 with npms 6 and 8. It's behaving correctly so far as I can tell and installing the package with this change on node 14 does work again.

@bkendall bkendall requested a review from yuchenshi September 30, 2022 16:38
@buu700
Copy link

buu700 commented Oct 2, 2022

I've also been running into issues with npm 8.19.2 and node 16.16.0, which this may have already fixed (not 100% sure). To reproduce the issue, run npm install firebase-tools esbuild ; npm ci; it will fail with an error message about a bunch of missing esbuild packages with version 0.15.8.

As a temporary workaround, I created a version of the package with the shrinkwrap file removed (firebase-tools@github:buu700/firebase-tools-tmp).

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