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

[REF-552] Reflex prints warning about Node binary not found #1778

Closed
masenf opened this issue Sep 7, 2023 · 3 comments
Closed

[REF-552] Reflex prints warning about Node binary not found #1778

masenf opened this issue Sep 7, 2023 · 3 comments
Labels
enhancement Anything you want improved error message Improve error message good first issue Good for newcomers

Comments

@masenf
Copy link
Collaborator

masenf commented Sep 7, 2023

Running reflex init inside a fresh python:3.11 docker contain prints the following warning twice

Warning: 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.

Which doesn't really make sense, because then it goes and bootstraps a version of node, which it then proceeds to use.

This warning should only be displayed if node is not installed, and Reflex is unable to bootstrap a version.

Present in 0.2.7a3

From SyncLinear.com | REF-552

@masenf masenf added enhancement Anything you want improved good first issue Good for newcomers error message Improve error message labels Sep 7, 2023
@masenf masenf changed the title Reflex prints warning about Node binary not found [REF-552] Reflex prints warning about Node binary not found Sep 7, 2023
@Billa05
Copy link
Contributor

Billa05 commented Oct 2, 2023

@masenf I have made some changes, now how to verify changes locally?

@masenf
Copy link
Collaborator Author

masenf commented Oct 2, 2023

@Billa05 please see https://github.com/reflex-dev/reflex/blob/main/CONTRIBUTING.md

@sgopalan98
Copy link

Hi @masenf

I looked at the source code for the problem that you mentioned. This is my understanding.

"reflex init" installs bun and node using processes.new_process function (called one time for each of their installation)

def new_process(args, run: bool = False, show_logs: bool = False, **kwargs):
"""Wrapper over subprocess.Popen to unify the launch of child processes.
Args:
args: A string, or a sequence of program arguments.
run: Whether to run the process to completion.
show_logs: Whether to show the logs of the process.
**kwargs: Kwargs to override default wrap values to pass to subprocess.Popen as arguments.
Returns:
Execute a child program in a new process.
"""
node_bin_path = path_ops.get_node_bin_path()
if not node_bin_path:
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."
)
# Add the node bin path to the PATH environment variable.

In this function, node's bin path is fetched in order to be added to the PATH variable, and if node isn't installed, the warning that you mentioned is thrown.

Since python3:11 docker image doesn't have node installed by default, and we need to install node using fnm zip, it makes sense for the initial calls to install bun and node to throw these warnings (as these installation is done through processes.new_process).

So, I feel that this warning makes sense - as until we install node, the warning needs to be thrown... Your thoughts on this?

@masenf masenf closed this as not planned Won't fix, can't repro, duplicate, stale Apr 3, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement Anything you want improved error message Improve error message good first issue Good for newcomers
Projects
None yet
Development

No branches or pull requests

3 participants