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

[BUG] Produced code invariably requires node:fs (dead code) #1809

Closed
xzxzzxzxzca opened this issue Jan 21, 2025 · 7 comments
Closed

[BUG] Produced code invariably requires node:fs (dead code) #1809

xzxzzxzxzca opened this issue Jan 21, 2025 · 7 comments
Labels

Comments

@xzxzzxzxzca
Copy link

Describe the bug
Let's say we have the simplest possible OCaml program:

let () =
  print_endline "Hello, World!"

Which we compile using jsoo:

(executable
 (name hello)
 (modes js)

 (modules hello)

 (js_of_ocaml
  (flags
    --opt 3
    --enable deadcode
    --enable globaldeadcode
  ))
)

Although the program performs no file operations, the generated code still requires node:fs and contains filesystem functions, even on the highest optimization level, even when asked to eliminate all dead code, even with --profile release.

This is extremely problematic for deployments to React Native platforms, where the module does not exist.

Expected behavior
Filesystem functions should not be included unless actually used.

As an alternative, the module could be loaded lazily.

Versions
ocamlc: 5.2.1
js_of_ocaml: 5.9.1

@hhugo
Copy link
Member

hhugo commented Jan 21, 2025

Try --target-env browser

@xzxzzxzxzca
Copy link
Author

@hhugo this indeed removes the dependency on node:fs (although, only with --profile release, otherwise still there, WUT).

However, this brings in references to window which, again, is not there in RN environment. RN is like node, but fewer standard modules.

However, this also creates issues with references to JS object implemented in Ocaml and accessed from another library's (javascript_files ...). Sorry, I can't immediately provide a minimal reproducer.

@hhugo
Copy link
Member

hhugo commented Jan 21, 2025

(although, only with --profile release, otherwise still there, WUT).

You need to set the flag when building the runtime
(build_runtime_flags (:standard --target-env browser))

@hhugo
Copy link
Member

hhugo commented Jan 21, 2025

Although the program performs no file operations,

Printing to stdout with print_endline uses node:fs.writeSync when running on node

@xzxzzxzxzca
Copy link
Author

Printing to stdout with print_endline uses node:fs.writeSync when running on node

Is there a way to somehow force non-inclusion of a module (e.g. node:fs) and a compilation error when one is required by the code?

Or is there a way to trace down which libraries/modules/functions pull in dependencies like the one we are talking about?

@hhugo

@hhugo
Copy link
Member

hhugo commented Jan 24, 2025

Is there a way to somehow force non-inclusion of a module (e.g. node:fs)

--target-env browser

and a compilation error when one is required by the code?

no

Or is there a way to trace down which libraries/modules/functions pull in dependencies like the one we are talking about?

manual inspection

However, this brings in references to window which, again, is not there in RN environment.

I don't know where this reference come from. I don't think it's related to --target-env browser

I don't know the constraints impose by react native. Maybe you can update jsoo to support --target-env react-native.

@xzxzzxzxzca
Copy link
Author

xzxzzxzxzca commented Jan 25, 2025

(although, only with --profile release, otherwise still there, WUT).

You need to set the flag when building the runtime (build_runtime_flags (:standard --target-env browser))

This, in combination with (flags --target-env nodejs), seems to solve the issue for me.
Thanks @hhugo I think we can close this for now

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

2 participants