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

Expose process.env.NODE_OPTIONS parser to user land #52709

Open
Ethan-Arrowood opened this issue Apr 26, 2024 · 2 comments
Open

Expose process.env.NODE_OPTIONS parser to user land #52709

Ethan-Arrowood opened this issue Apr 26, 2024 · 2 comments
Assignees
Labels
c++ Issues and PRs that require attention from people who are familiar with C++. feature request Issues that request new features to be added to Node.js. lib / src Issues and PRs related to general changes in the lib or src directory. util Issues and PRs related to the built-in util module.

Comments

@Ethan-Arrowood
Copy link
Contributor

Ethan-Arrowood commented Apr 26, 2024

What is the problem this feature will solve?

Node.js has an internal parser for correctly parsing and validating the process.env.NODE_OPTIONS variable. I'd like to expose this parser to user land as something like util.parseNodeOptions.

What is the feature you are proposing to solve the problem?

The exact parser code is here:

std::vector<std::string> ParseNodeOptionsEnvVar(

The solution should create a new C++ function that accesses the NODE_OPTIONS environment variable from the Environment so that we do not incur an unnecessary serialization cost passing the NODE_OPTIONS from JS.

Calling the function should return the list of strings.

The function should throw any error it encounters during the parsing process.

I will contribute this feature myself soon 👍

What alternatives have you considered?

We required this functionality for a recent bug in Next.js. I was able to solve that issue by manually converting the C++ parser to JS and using that.

The PR introducing the fix: vercel/next.js#65046

The JS parser implementation itself: https://github.com/vercel/next.js/blob/270a9db0567808db5d53e7a18c7be0a4710796e8/packages/next/src/server/lib/utils.ts#L59-L113

@Ethan-Arrowood Ethan-Arrowood added the feature request Issues that request new features to be added to Node.js. label Apr 26, 2024
@Ethan-Arrowood Ethan-Arrowood self-assigned this Apr 26, 2024
@Ethan-Arrowood Ethan-Arrowood added util Issues and PRs related to the built-in util module. c++ Issues and PRs that require attention from people who are familiar with C++. lib / src Issues and PRs related to general changes in the lib or src directory. labels Apr 26, 2024
@joyeecheung
Copy link
Member

Note that ParseNodeOptionsEnvVar only splits the argument string into a vector of strings by whitespaces. It's not a full parser (i.e. doesn't validate and assign values).

@Ethan-Arrowood
Copy link
Contributor Author

That is okay. I think providing even the consistent whitespace/quote parsing is important. The core of the issue we ran into in Next.js was because we didn't understand the parsing rules for process.env.NODE_OPTIONS.

I'd have to go and look at the code again, but there was something else that was validating the parsed options. That could be valuable to export as well.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
c++ Issues and PRs that require attention from people who are familiar with C++. feature request Issues that request new features to be added to Node.js. lib / src Issues and PRs related to general changes in the lib or src directory. util Issues and PRs related to the built-in util module.
Projects
Status: Pending Triage
Development

No branches or pull requests

2 participants