Skip to content

Exclude VS Code properties from environment variables#8580

Closed
d13 wants to merge 1 commit intopnpm:mainfrom
d13:fix-blacklist-properties-from-package-json
Closed

Exclude VS Code properties from environment variables#8580
d13 wants to merge 1 commit intopnpm:mainfrom
d13:fix-blacklist-properties-from-package-json

Conversation

@d13
Copy link
Copy Markdown

@d13 d13 commented Sep 27, 2024

See #8552


For more details, open the Copilot Workspace session.

@welcome
Copy link
Copy Markdown

welcome bot commented Sep 27, 2024

💖 Thanks for opening this pull request! 💖
Please be patient and we will get back to you as soon as we can.

@zkochan
Copy link
Copy Markdown
Member

zkochan commented Sep 28, 2024

If I understand correctly, we automatically add all fields from package.json to env variables. If that is the case, we need to check what field does npm include and use the same list.

@zkochan
Copy link
Copy Markdown
Member

zkochan commented Sep 28, 2024

I believe it happens in this function: https://github.com/pnpm/npm-lifecycle/blob/1e0f149176877f906968a01087cf63290652ccbc/index.js#L347

See all the env variables created with the npm_package_ prefix. We should find how npm does it. Probably it has an allowlist of fields.

@d13
Copy link
Copy Markdown
Author

d13 commented Sep 30, 2024

@zkochan I will dig into that, thanks!

@eamodio
Copy link
Copy Markdown

eamodio commented Oct 2, 2024

It looks like it is here:

https://github.com/npm/cli/blob/6ca609e20b68fb2e5ef8177db116b84a339461fd/workspaces/arborist/lib/arborist/rebuild.js#L311-L320

I also verified that on my Windows box that those are the only npn_package_ prefixed env variables I see when a script is run

@zkochan
Copy link
Copy Markdown
Member

zkochan commented Dec 9, 2024

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