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

Declarative syntax as the received way to access private state is highly valuable #3

Open
syg opened this issue Sep 7, 2018 · 14 comments

Comments

@syg
Copy link

syg commented Sep 7, 2018

I see the declarative syntax of class fields and that class field accesses use . instead of computed property accesses as pros of the class fields proposal.

Declarative syntax and static property accesses are easier to reason about both for the human and the compiler. And code that deals with private state is more often than not the code that I'd like to be most sure of as having its invariants intact.

@joyeecheung
Copy link

joyeecheung commented Sep 12, 2018

In Node.js core we have a guideline on using symbols to hide private states (they are not private but there are no better alternatives), because we have caught numerous usage of underscore-prefixed properties/methods in code reviews and have to request changes to use symbol properties instead (Node.js has many existing underscore properties that can't be removed easily out of compatibility concerns so we can't enforce the style with a linter). A declarative syntax would send a clearer signal to developers that this is something they should use to access private state, instead of going with underscores (or nothing at all) to save a few more keystrokes.

A project like Node.js may have enough reviewers to catch that kind of usage, but projects with fewer eyeballs have a higher risk of leakier code just because symbols are less ergonomic. Even with the amount of reviewers we have, not everyone is aware of this style as sometimes new collaborators or collaborators who have been absent for a while come and approve patches that leak private state until someone request changes to use symbols. Private symbols provide the means to hide private state, but it does not provide as enough incentive as a declarative syntax would.

@zenparsing
Copy link
Owner

Hi @joyeecheung,

Platform and framework authors are definitely the target audience for this feature, so it's important that it works well for the Node.js internals use case.

I'm curious about this statement:

Node.js has many existing underscore properties that can't be removed easily out of compatibility concerns so we can't enforce the style with a linter

Yes, I've used underscore properties from "module" many times. 😄

Would it be possible to annotate the source code with comments, indicating which underscore properties are OK from the linter's point of view?

@joyeecheung
Copy link

Would it be possible to annotate the source code with comments, indicating which underscore properties are OK from the linter's point of view?

I just did a quick search for ._ in Node.js's lib folder (where the JS code live) on the master branch and there are 1707 results in 82 files - there could be more underscore properties that are accessible but not declared/used this way (e.g. declared using the ES2015 class syntax). It's certainly not impossible to annotate them all, but it would be a big haul, and may not help with code reviews because with that many existing properties, it still requires a human to tell if one added in a diff is overriding/moving an existing one, or is indeed a new private property that should be hidden behind symbols.

@zenparsing
Copy link
Owner

Thanks @joyeecheung. I can see how linting would be a bit of a headache for that scenario.

@zenparsing
Copy link
Owner

A couple of notes on syntax:

First, syntax can be layered on top of Symbol.private, even perhaps the same #-name syntax of the fields proposal. I'm not necessarily opposed to that!

We would likely want to look at whether there are some syntax tweaks that would better align with private symbols though. We've seen proposals for private #x outside of classes, which is thinking along the right lines but is also problematic for various reasons. We might also want to look into whether syntactic sugar for all Symbols makes sense.

Also, I think there's a valid argument that says that this[foo] is good enough for the core use cases. Certainly, the square brackets make this more of an advanced feature, but hard privacy is an advanced feature. Most application code, and probably most library code, works just fine with underscores.

I know that Node doesn't use TypeScript currently, but there's another potential solution there: possibly we can add a option that instructs the compiler to generate private symbols for private class members. Here too, the semantics of private symbols make this kind of transformation easy and straightforward.

In short, I'm not too worried about syntax.

@syg
Copy link
Author

syg commented Sep 14, 2018

First, syntax can be layered on top of Symbol.private, even perhaps the same #-name syntax of the fields proposal. I'm not necessarily opposed to that!

Certainly. What I intended for this issue was that value is lost if syntax becomes future work instead of part of the core proposal.

Also, I think there's a valid argument that says that this[foo] is good enough for the core use cases. Certainly, the square brackets make this more of an advanced feature, but hard privacy is an advanced feature. Most application code, and probably most library code, works just fine with underscores.

I don't have the intuition that hard privacy is an advanced feature. This proposal is an advanced feature, but I'd like hard privacy for the common case to not be an advanced feature, and I feel like the current class fields proposal does provide that better than private symbols do.

@Igmat
Copy link

Igmat commented Sep 28, 2018

@syg and what do you think about syntax sugar on top of private-symbols described here: tc39/proposal-class-fields#134 ?

@syg
Copy link
Author

syg commented Sep 28, 2018

@Igmat I'd like to to avoid debating the particulars of syntax and alternatives here, but for the record I do like the current # name syntax. The intention of this issue was to point out that there is a difference between symbols-with-syntax-layered-on vs syntax-with-a-runtime-escape-hatch, not to only lament the lack of syntax. The first approach is what private symbols proposes, and the second approach is the status quo of the class fields proposal. My preference is for the second approach for the reasons above.

@Igmat
Copy link

Igmat commented Sep 28, 2018

@syg and why do you prefer security over metaprogramming, excluding the fact that existing approach already in stage 3?

@syg
Copy link
Author

syg commented Sep 28, 2018

@Igmat I don't understand the framing of that question.

@Igmat
Copy link

Igmat commented Sep 28, 2018

tc39/proposal-class-fields#106 (comment)
this is latest answer to problem with current proposal and proxies, at short it means you have to choose one of Proxy or #-private, not both at same time.
Which dramatically decrease amount of possible metaprogramming scenarios that could be used with proxies (e.g. truly reactive properties).
While private-symbol-proposal doesn't have such issue with proxies, it provides another trade-off:
you can't use brand-checking that based only on privates without additional code.

So, since the only reasonable scenario for brand-checking is some security stuff (e.g. passing objects to some untrusted execution context) and could be achieved using existing tools (e.g. WeakSet/WeakMap), I'm asking you why do you prefer Security over Metaprogramming?

@bathos
Copy link

bathos commented Sep 28, 2018

the only reasonable scenario for brand-checking is some security stuff

Security is not the only reason to perform brand checks, but I don’t think it’s a significant factor since, as you said, it can be achieved regardless of how private <whatevers> work.

@Igmat
Copy link

Igmat commented Sep 28, 2018

@bathos just for curiosity, what other scenarios for brand-checking I missed?

@bathos
Copy link

bathos commented Sep 28, 2018

Polyfilling APIs defined by Web IDL is probably the most common, but also just in ordinary library code that wishes to establish very strict invariants. Just as one might be aggressive about runtime type checking of ordinary arguments; this is effectively an argument passed by other means.

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

No branches or pull requests

5 participants