Skip to content

Honor Nodejs environment variables #24336

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

Open
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

turran
Copy link

@turran turran commented May 15, 2025

Be able to pass Node.js environment variables through the command line and make them available to Emscripten

@kleisauke
Copy link
Collaborator

This is a breaking change, see e.g. the discussion in PR #18820.

@turran
Copy link
Author

turran commented May 15, 2025

Thanks @kleisauke I wasn't aware of that PR.
I don't understand why such a change can be a breaking change. Not honoring node env is a missing feature as there is no way to pass environment variables other than manually modifying the .js file, which is impractical from the user PoV.

@sbc100
Copy link
Collaborator

sbc100 commented May 15, 2025

Thanks @kleisauke I wasn't aware of that PR. I don't understand why such a change can be a breaking change. Not honoring node env is a missing feature as there is no way to pass environment variables other than manually modifying the .js file, which is impractical from the user PoV.

Its a little more subtle than that. Emscripten tends to present a kind of virtual environment. By default for example, we don't expose the user's actual filesystem, even under node. You have to opt into that behaviour. See https://github.com/emscripten-core/emscripten/pull/18820/files#r1114735936

I do think we want to enable this behaviour somehow, but it will likely need to be behind some kind of flag.

In any case this does appear to be a duplicate of #18820, no?

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

Successfully merging this pull request may close these issues.

3 participants