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

Should this also censor .name? #2

Closed
Jamesernator opened this issue Jan 15, 2018 · 10 comments
Closed

Should this also censor .name? #2

Jamesernator opened this issue Jan 15, 2018 · 10 comments

Comments

@Jamesernator
Copy link

My inclination is no but it seems similar in that the name might be used in strange ways by a caller prevent changing the name of the original function.

For example if we have an object like { render() { ... }, update() { ... } } then if a consumer of that object uses the .name property as a key then the object could never be changed to { render: definedOutsideObject, update: debounce(function update() {}) }.

I don't know if this has even been in common usage like it was for parameter parsing in angular so it might not be worthwhile.

@domenic
Copy link
Member

domenic commented Jan 15, 2018

Good question. I think no since you can already do censorship of name using Object.defineProperty. But I'll add a section to the README addressing this question.

@michaelficarra
Copy link
Member

@domenic I'm not sure "can already do this" is sufficient to make this decision. Remember you could say the same about hiding toString results as I've demonstrated. For named function expressions or function declarations declared in a private scope, I imagine an author would consider those names almost like comments, or at least like local variable names. It's possible that many developers may even be surprised that the names of their functions are exposed at all.

On a related note, I want to consider length censorship at the same time. Changing the number of mandatory parameters (especially when replacing a mandatory parameter with a defaulting or rest parameter) should be an implementation detail, not a part of the public API.

@domenic Can we re-open for further discussion?

@domenic domenic reopened this Mar 15, 2019
@domenic
Copy link
Member

domenic commented Mar 15, 2019

Happy to reopen for further discussion, but I'm a bit unsure on the use case here.

As you might imagine, I'm working on analogy to built-ins, whose implementation details are hidden in the way I'm hoping that our userland libraries can attain. There:

  • Functions that are accessible from the outside have useful .name properties reflecting the name the author gave them
  • Functions that are accessible from the outside have useful .length properties reflecting the number of parameters the author declared them with
  • For functions that are not accessible from the outside, such as function declarations in private scope, it doesn't matter, since you can't get access to the object to look at its .length or .name properties.
    • Note that in the context of this proposal, that would hold even in stack traces, since such non-accessible functions would be hidden from stack traces.

Can you say more about the case you're concerned about, perhaps with code samples?

@ljharb
Copy link
Member

ljharb commented Mar 15, 2019

imo the number of mandatory parameters is an inextricable part of the public API.

@michaelficarra
Copy link
Member

The original motivation for this proposal included both reliably polyfilling built-ins and easing refactoring/encapsulation.

Function names being provided gives unwanted extra implementation information:

(function(){
  function a() { ... }
  function b() { ... }
  f(someDecision() ? a : b)
}())

I don't want f to know which of a or b that it has received. This also hurts my ability to refactor. Say I wanted to give the functions meaningful names:

(function(){
  function meaningfulNameA() { ... }
  function meaningfulNameB() { ... }
  f(someDecision() ? meaningfulNameA : meaningfulNameB)
}())

Similarly, if my library provides an API:

export function doSomething(a, b) { ... }
export function doSomethingElse(a, b) { ... }

I cannot refactor that API to use default parameters or rest parameters, even if they would otherwise provide the same functionality.

export function doSomething(a, b = 0) { ... }
export function doSomethingElse(a, ...b) { ... }

So in these examples I've shown that implicitly providing name and length with all functions can cause difficulties for library authors when performing refactoring.

It's important to note, though, that this goal conflicts to some degree with the polyfilling goal since an accurate polyfill would want to match the name and length of the built-in it is meant to emulate.

@domenic
Copy link
Member

domenic commented Mar 15, 2019

Yes, I agree there's a conflict of goals here.

I don't want f to know which of a or b that it has received. This also hurts my ability to refactor.

My take is that passing functions from a library to outside code as callbacks, and not wanting the caller to know these function names, is more rare than a library exposing methods/classes/functions to outside code, and wanting that outside code to know those function names. So I am OK making the callback case require wrapping, and not automatically get censored via the "hide implementation" pragma.

Similarly, if my library provides an API:

This case is a more stark example of conflict. In particular, you're claiming that the length property of the function is just an implementation detail, whereas from my perspective (and @ljharb's) it's part of the public API.

To be clear, I don't mean that it's public API in the trivial sense of "it's publicly exposed". A function's .toString() value is a public API, but the original point of this repository is to say that it should not be, at least when the author doesn't want it to be. I mean that we actively design APIs, both in userspace and in the platform, with an eye toward communicating useful information via the .length and .name.

(This is clearest for .name, BTW: code that switches on .constructor.name is very common, and I want to be able to author libraries which hide their source code but still expose useful values for .constructor.name.)

So overall, to me it makes more sense to be conservative here, and focus on the intersection use cases. Maybe we want a second pragma for .length/.name hiding?

@michaelficarra
Copy link
Member

So overall, to me it makes more sense to be conservative here, and focus on the intersection use cases. Maybe we want a second pragma for .length/.name hiding?

Okay yeah, it seems they'd have to be separated. I think this would be an interesting topic for the next time this proposal is presented to committee.

@michaelficarra
Copy link
Member

The committee chose not to add a convenient way to do this.

@codehag
Copy link

codehag commented Dec 2, 2019

@michaelficarra can you give some information about why the committee decided this? The last notes i found on this were here: https://github.com/tc39/notes/blob/master/meetings/2019-03/mar-27.md#function-implementation-hiding-stage-2-update but they are a bit hard to follow.

Also the proposal was recently discussed here https://github.com/tc39/notes/blob/master/meetings/2019-07/july-24.md#update-on-function-implementation-hiding but it doesnt seem to be related to this

@michaelficarra
Copy link
Member

@codehag Sorry, I don't have the details anymore. I can bring it up again at the next presentation to committee. But I haven't advocated for this feature since we decided which use cases we would consider for this proposal. Hiding name/length is unwanted for the built-in polyfilling use case, which would complicate the solution. There's also a fairly easy way to do it today using Object.defineProperty for each of name/length.

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