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

process: use frozen array for process.allowedNodeEnvironmentFlags #37171

Closed
wants to merge 4 commits into from

Conversation

aduh95
Copy link
Contributor

@aduh95 aduh95 commented Feb 1, 2021

Maintain no-op calls of Set prototype methods to process.allowedNodeEnvironmentFlags for backward compatibility.

Labeled as semver-major as it makes following code throw:

Set.prototype.add.call(process.allowedNodeEnvironmentFlags, '--user-option`);
process.allowedNodeEnvironmentFlags.has('--user-option') === true;

Originally suggested by @Trott in #36660 (review).

Maintain no-op calls of `Set` prototype methods to
`process.allowedNodeEnvironmentFlags` for backward compatibility.
@aduh95 aduh95 added semver-major PRs that contain breaking changes and should be released in the next major version. process Issues and PRs related to the process subsystem. labels Feb 1, 2021
@benjamingr
Copy link
Member

I don't like having a weird set-like-array-object around - wouldn't it be simpler to subclass SafeSet in a way that noops on mutations?

@benjamingr
Copy link
Member

Oh, I see that's what the current code does, then the issue is getting external set prototype calls on the set mutating it?

@aduh95
Copy link
Contributor Author

aduh95 commented Feb 1, 2021

Yes, I did a proposal in #36660 to circumvent the direct Set prototype calls using subclassing, and the suggestion from Rich was to use a frozen object instead (this PR).

@aduh95 aduh95 added the stalled Issues and PRs that are stalled. label Mar 9, 2021
@github-actions
Copy link
Contributor

github-actions bot commented Mar 9, 2021

This issue/PR was marked as stalled, it will be automatically closed in 30 days. If it should remain open, please leave a comment explaining why it should remain open.

@aduh95 aduh95 closed this May 15, 2021
@aduh95 aduh95 deleted the frozen-process-allowedEnvFlags branch May 15, 2021 22:40
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
process Issues and PRs related to the process subsystem. semver-major PRs that contain breaking changes and should be released in the next major version. stalled Issues and PRs that are stalled.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants