Skip to content

support passing NODE_PATH as list#78

Merged
SteffenDE merged 3 commits intomainfrom
sd-path-sep
May 27, 2025
Merged

support passing NODE_PATH as list#78
SteffenDE merged 3 commits intomainfrom
sd-path-sep

Conversation

@SteffenDE
Copy link
Copy Markdown
Contributor

No description provided.

Copy link
Copy Markdown
Member

@josevalim josevalim left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think we should normalize all entries that are lists, not only NODE_PATH. WDYT?

@SteffenDE
Copy link
Copy Markdown
Contributor Author

I'm not sure, since this separator seems to be specific to PATH-like variables. Claude says:

Q: I'm adding a feature to an Elixir library to pass a list of strings to a CLI program's ENV variable NODE_PATH. It joins strings with ":" on unix and ";" on Windows. Is it reasonable to apply this joining to all environment variables configured as a list?

It's not reasonable to apply this joining logic to all list environment variables by default. Here's why:

Path-specific Convention

The colon/semicolon delimiter pattern is specifically for PATH-like variables:

  • PATH
  • NODE_PATH
  • PYTHONPATH
  • CLASSPATH
  • LD_LIBRARY_PATH
    These all represent filesystem paths and follow the OS-specific path separator convention.

Other List Formats Exist

Many environment variables use different delimiters:

  • Comma-separated: SOME_FLAGS=opt1,opt2,opt3
  • Space-separated: CFLAGS=-O2 -Wall -Wextra
  • Custom delimiters: DATABASE_URLS=postgres://host1|postgres://host2
  • No delimiter (concatenated): Some variables expect values without separators

@SteffenDE
Copy link
Copy Markdown
Contributor Author

SteffenDE commented May 27, 2025

I've been thinking that maybe instead of applying this to env: %{"NODE_PATH" => ...}, we could also just add a config

config :esbuild,
  profile: [
    ...,
    node_paths: []
  ]

and set NODE_PATH internally?

@josevalim
Copy link
Copy Markdown
Member

I'm not sure, since this separator seems to be specific to PATH-like variables.

Exactly, but we can assume that anything given as a list is meant to be a PATH-like variable.

We could also set a special :node_path profile... but it comes with some downsides in that, if someone reads esbuild official docs, now they need to know there is special profile option and what it means.

@SteffenDE SteffenDE merged commit c891ea2 into main May 27, 2025
2 checks passed
@SteffenDE
Copy link
Copy Markdown
Contributor Author

🙌🏻

@josevalim josevalim deleted the sd-path-sep branch May 27, 2025 11:13
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.

2 participants