[REF-2086] Avoid "Warning: The path to the Node binary could not be found."

When bootstrapping node itself, we do not need to warn the user that node
cannot be found -- it's obviously not found because we're installing it right
now. This warning leads some folks to believe that they need to install node
themselves (for example in a docker container), when in reality, reflex will
always try to install it's own known-good version of node first.

Also avoid a nasty traceback when Reflex attempts to run node or npm, but it's
not actually installed (instead, the warning will be displayed and it will say
"Invalid Command").

Move the recently added check for node before installing node above the
conditional so it can also skip node installation on Windows, which is
especially slow.
This commit is contained in:
Masen Furer 2024-03-06 17:50:36 -08:00
parent 8903ebb8b0
commit ef457afe11
No known key found for this signature in database
GPG Key ID: B0008AD22B3B3A95
2 changed files with 24 additions and 7 deletions

View File

@ -34,6 +34,8 @@ from reflex.compiler import templates
from reflex.config import Config, get_config
from reflex.utils import console, path_ops, processes
CURRENTLY_INSTALLING_NODE = False
def check_latest_package_version(package_name: str):
"""Check if the latest version of the package is installed.
@ -103,8 +105,11 @@ def get_node_version() -> version.Version | None:
Returns:
The version of node.
"""
node_path = path_ops.get_node_path()
if node_path is None:
return None
try:
result = processes.new_process([path_ops.get_node_path(), "-v"], run=True)
result = processes.new_process([node_path, "-v"], run=True)
# The output will be in the form "vX.Y.Z", but version.parse() can handle it
return version.parse(result.stdout) # type: ignore
except (FileNotFoundError, TypeError):
@ -606,6 +611,11 @@ def install_node():
console.debug("")
return
# Skip installation if check_node_version() checks out
if check_node_version():
console.debug("Skipping node installation as it is already installed.")
return
path_ops.mkdir(constants.Fnm.DIR)
if not os.path.exists(constants.Fnm.EXE):
download_and_extract_fnm_zip()
@ -622,10 +632,6 @@ def install_node():
],
)
else: # All other platforms (Linux, MacOS).
# TODO we can skip installation if check_node_version() checks out
if check_node_version():
console.debug("Skipping node installation as it is already installed.")
return
# Add execute permissions to fnm executable.
os.chmod(constants.Fnm.EXE, stat.S_IXUSR)
# Install node.
@ -926,8 +932,12 @@ def initialize_frontend_dependencies():
"""Initialize all the frontend dependencies."""
# validate dependencies before install
validate_frontend_dependencies()
# Avoid warning about Node installation while we're trying to install it.
global CURRENTLY_INSTALLING_NODE
CURRENTLY_INSTALLING_NODE = True
# Install the frontend dependencies.
processes.run_concurrently(install_node, install_bun)
CURRENTLY_INSTALLING_NODE = False
# Set up the web directory.
initialize_web_directory()

View File

@ -135,13 +135,20 @@ def new_process(args, run: bool = False, show_logs: bool = False, **kwargs):
Returns:
Execute a child program in a new process.
Raises:
Exit: When attempting to run a command with a None value.
"""
node_bin_path = path_ops.get_node_bin_path()
if not node_bin_path:
if not node_bin_path and not prerequisites.CURRENTLY_INSTALLING_NODE:
console.warn(
"The path to the Node binary could not be found. Please ensure that Node is properly "
"installed and added to your system's PATH environment variable."
"installed and added to your system's PATH environment variable or try running "
"`reflex init` again."
)
if None in args:
console.error(f"Invalid command: {args}")
raise typer.Exit(1)
# Add the node bin path to the PATH environment variable.
env = {
**os.environ,