Fnm and node for POSIX #1606
Fnm and node for POSIX #1606
Conversation
|
should be reviewed/merged after #1566 |
ElijahAhianyo
force-pushed
the
elijah/posix_fnm
branch
from
94e89ad to
670050c
Compare
ElijahAhianyo
force-pushed
the
elijah/posix_fnm
branch
from
e6c99a6 to
76e139e
Compare
reflex/utils/prerequisites.py
Outdated
| # fnm only support Linux, macOS and Windows distros. | ||
| return |
There was a problem hiding this comment.
so what will happen in this case? can the user install and run their own copy of node in these environments? asking because it looks like we set NODE_BIN_PATH in an FNM-specific way
There was a problem hiding this comment.
Thats a good question, Previously(when users had to install node themselves), we checked for node on the host machine (which node or use the node command and check for errors) and used that. We can do that as well as expose a config knob (just like we do for bun) to allow users specify custom paths to node binaries, but Im open to suggestions. What do you think?
cc: @masenf @picklelo @Lendemor @jackie-pc
There was a problem hiding this comment.
i think we should fall back to using shutil.which() if fnm is not available. Down the road we might allow users to "bring your own node", but for now, lets keep it simple as a fallback when our installation mechanism fails.
There was a problem hiding this comment.
the which solution sounds reasonable to me. Should we log something (even if debug level) when we fall through to such cases. I.e. either we use which node or straight give up. Note this should be a pretty rare case and so it's worth logging something. I.e. we are talking about an environment that is NOT Windows, Linux or Mac (roughly)
| @@ -18,6 +19,28 @@ | |||
| IS_WINDOWS = platform.system() == "Windows" | |||
|
|
|||
|
|
|||
| def get_fnm_name() -> Optional[str]: | |||
There was a problem hiding this comment.
I think this function can be moved to prerequisites.py
| # The directory to store fnm. | ||
| FNM_DIR = os.path.join(REFLEX_DIR, "fnm") | ||
| FNM_FILENAME = get_fnm_name() |
There was a problem hiding this comment.
Can be moved to prerequisites.py
There was a problem hiding this comment.
The FNM_FILENAME is needed to construct the URL to the release binary (FNM_INSTALL_URL). Do you mean to move that as well (i.e move get_fnm_name, FNM_FILE_NAME and FNM_INSTALL_URL to prerequisites)?
| if platform_os == "Windows": | ||
| return "fnm-windows" | ||
| elif platform_os == "Darwin": | ||
| return "fnm-macos" |
There was a problem hiding this comment.
Why is there no ARM vs Intel distinction for Windows and Mac?
There was a problem hiding this comment.
i would assume macos is using fat binaries and windows is probably falling back to x86 emulation... but yeah, would definitely be good to test. i have windows arm, but i don't have x86 mac to test on.
There was a problem hiding this comment.
Not sure about Windows, but looking at their install script, I would assume the binaries for ARM are specific to LInux.
reflex/utils/prerequisites.py
Outdated
| # fnm only support Linux, macOS and Windows distros. | ||
| return |
There was a problem hiding this comment.
the which solution sounds reasonable to me. Should we log something (even if debug level) when we fall through to such cases. I.e. either we use which node or straight give up. Note this should be a pretty rare case and so it's worth logging something. I.e. we are talking about an environment that is NOT Windows, Linux or Mac (roughly)
Description
This pull request aims to enhance the installation process of Node.js by replacing the previous usage of NVM with FNM. This change not only introduces cross-platform compatibility but also leverages FNM's capability to generate release binaries for Linux, macOS, and Windows distributions. Furthermore, the installation of dependencies has been streamlined to align with platform conventions.
Changes Made
C:/Users/<username>/AppData/Local/reflex~/Library/Application Support/reflex~/.local/share/reflexAdditional Context
The motivation behind this PR is to provide a unified and reliable installation experience for Node.js across different operating systems. The transition from NVM to FNM enables seamless management of Node.js versions while addressing Windows compatibility limitations that existed with NVM. The adoption of platform-specific directories ensures clean separation of dependencies, enhancing organization and system integration.
It's worth noting that FNM generates release binaries for Linux, macOS, and Windows distributions, which aligns perfectly with the multi-platform approach of this project.
This PR builds upon the previous Windows-related PR #1566 (merged for v0.2.5), and its successful testing and merging indicate that this solution is ready for wider adoption.
Closes #1585