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

Add option for Nodejs path like BUN_PATH #3905

Closed
kraanzu opened this issue Sep 10, 2024 · 7 comments · Fixed by #4006
Closed

Add option for Nodejs path like BUN_PATH #3905

kraanzu opened this issue Sep 10, 2024 · 7 comments · Fixed by #4006
Labels
bug Something isn't working

Comments

@kraanzu
Copy link

kraanzu commented Sep 10, 2024

Describe the bug
Reflex tries to install the required node version by itself into data directory but this fails on systems like NixOS or Guix because its not allowed to run binaries directly

NixOS cannot run dynamically linked executables intended for generic
linux environments out of the box. For more information, see:
https://nix.dev/permalink/stub-ld

To Reproduce

  • reflex init and choose any template
  • reflex run

Expected behavior
Reflex runs without any errors just like in the documentation

Specifics (please complete the following information):

  • Python Version: Python 3.12.5
  • Reflex Version: 0.5.10
  • OS: NixOS 22.04

Additional context
Related issue: #1470

@kraanzu kraanzu added the bug Something isn't working label Sep 10, 2024
@Lendemor
Copy link
Collaborator

Lendemor commented Sep 10, 2024

Here is the function we run when searching for node.

def get_node_path() -> str | None:
    """Get the node binary path.

    Returns:
        The path to the node binary file.
    """
    node_path = Path(constants.Node.PATH)
    if not node_path.exists():
        return str(which("node"))
    return str(node_path)

So if node is missing from the reflex data folder, it should try to find wherever you have it installed at the moment.

@kraanzu
Copy link
Author

kraanzu commented Sep 11, 2024

Hmm... I checked and it seems to return a path that is OK

In [2]: get_node_path()
Out[2]: '/run/current-system/sw/bin/node'

In my case, The data directory is ~/.local/share/reflex/, and if I were to delete this, then it should use the which("node") but that is not the case, it re-installs the node binary and then the function above returns something different

In [2]: get_node_path()
Out[2]: '/home/kraanzu/.local/share/reflex/fnm/node-versions/v18.17.0/installation/bin/node'

and hence the error. I'll do a bit more digging on this

@kraanzu
Copy link
Author

kraanzu commented Sep 11, 2024

I think this function is the issue while deciding whether to install node or not:

def check_node_version() -> bool:
    """Check the version of Node.js.

    Returns:
        Whether the version of Node.js is valid.
    """
    current_version = get_node_version()
    if current_version:
        # Compare the version numbers
        return (
            current_version >= version.parse(constants.Node.MIN_VERSION)
            if constants.IS_WINDOWS
            else current_version == version.parse(constants.Node.VERSION)
        )
    return False

I'm currently using node version 18.20.4 which was the last version before they moved to 19.x

The function above checks for total equality which causes the problem here, even the patch version update will return a False and will result in installing Node separately

@Lendemor
Copy link
Collaborator

#3981 no longer solve this issue, but a follow up PR will.

@kraanzu
Copy link
Author

kraanzu commented Sep 26, 2024

I also saw another mentioned PR which changed how the nodejs binary is checked, is reflex now supporting latest nodejs ?

@Lendemor
Copy link
Collaborator

With PR #4006, we've added a CI check to run the integration test with latest node (22.9.0 at the time of writing this, but CI will always check the latest one.)

You can either try the PR or wait for 0.6.1 which should include this fix.

export REFLEX_USE_SYSTEM_NODE=1 will make sure that Reflex use the node found at which node instead of the fnm installed one.

@kraanzu
Copy link
Author

kraanzu commented Sep 26, 2024

Awesome. Thanks a lot

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants