Clarification on the use case of protecting against callers inspecting functions' parameter names

[misterdjules]

This proposal mentions the following:

A historical example of this in action is AngularJS's reliance on f.toString() to inspect the parameter names of a function. It would then use these as part of its dependency injection framework. This made what was previously a non-breaking change — modifying the name of a parameter — into a breaking one. In particular, this kind of usage of Function.prototype.toString makes it impossible to use otherwise-semantics-preserving minification tools in combination with this mode of AngularJS.

I've never used Angular, so this example is a bit difficult for me to understand. After reading this paragraph, I'm wondering:

1. why any change to callees break the caller (in this case Angular) if the caller is able to inspect the callee's parameters name. If the callee's parameters' names change, wouldn't the caller be able to detect that and adapt its behavior?

2. why instead of hiding implementation details to work around this problem, we wouldn't make introspecting callee's parameters a first class API of the language.

[ljharb]

@misterdjules in angular's case, i believe the name of the argument was keyed to injected dependencies. In other words, how do you programmatically adapt a to b?

[misterdjules]

@ljharb I don't think I understand what "the name of the argument was keyed to injected dependencies" means in practice. Having a code example to illustrate this issue seems like it would help clarify the associated use case.

[bathos]

@misterdjules In Angular 1, in certain circumstances Angular would look at the source text reported by toString on a user-defined function and extract the parameter names. These names had to match the names of previously registered services. It would retrieve those services (possibly with lazy instantiation) and invoke the function with them.

So for example Angular would invoke function($location) { ... } with a registered service named $location as the first argument. A minifier would change that argument name by default, but the service would still have been registered with the name "$location".

It was pretty quickly realized that this out-of-band channel was too clever & a misuse and shouldn’t have been promoted as something reliable. Not only minifiers but also more general compilation tooling and new ES features like arrow functions and default parameter values broke its behavior. The magic was deprecated in favor of explicit annotation.

[misterdjules]

The magic was deprecated in favor of explicit annotation.

Does that mean that "current" versions of angular (e.g. Angular 8) are not vulnerable to this problem?

[bathos]

Yes, afaik it only existed in Angular 1.

Angular 1 is still around and the feature still exists, though it can be disabled with the strictDi option. To understand why NG1 experimented in this space, it helps to remember that at the time the kinds of compilation steps which are common with JS today were still relatively rare. More modern tooling would tend to use things like decorators that get compiled to other code for this sort of thing (and NG2+ does use decorators).

In any case, this proposal wouldn’t impact existing code. Someone using ‘name inference’ dependency injection would have to actually introduce these directives to their code. I think the example was included as a historical illustration of a library interpreting exposed source as having semantics that aren’t really ‘there,’ and how that can go wrong. Angular 1’s name inference is probably the most famous example of this sort of reliance on an ostensibly out-of-band channel — or at least it was until React introduced ‘hooks’ :)

[misterdjules]

Thanks for the detailed info!

I think what confused me was that when reading the proposal, it seemed like the issue with Angular 1 was used as part of the motivation for why the proposed directives are needed, but it seems that folks solved this issue by not relying on implementation details, which seems like a better overall approach to me.

As a result I find it a bit misleading to keep this paragraph in the proposal and use it as a motivation for it.

[michaelficarra]

the example was included as a historical illustration of a library interpreting exposed source as having semantics that aren’t really ‘there,’ and how that can go wrong

@misterdjules The only way in which the example is misleading is that it's "backwards" from the use cases we've put forward. In our use cases, we are concerned with bad consumers introspecting on library innards, but in the example, it was the library that was behaving badly.

it seems that folks solved this issue by not relying on implementation details, which seems like a better overall approach to me.

It's true that the problem this proposal seeks to solve can be resolved by people not relying on implementation details, but in fact people do rely on implementation details, even if they eventually realize that they shouldn't. It's best if the language provides a clean way of not exposing details that you do not wish to expose so that the problem can be avoided in the first place.

[voliva]

I think the whole "Problem" section in the README needs clarification. After reading it, I don't fully understand how the current proposal would help on any of the problems that section describes.

In general, the problem described is the issue on Function.prototype.toString() by itself, but this proposal will not solve it. It will discourage using it on some functions, the ones that opt-in to this.

To be specific:

• Relying on the internals of what Function.prototype.toString() returns is a hack, and should always be considered at-your-own-risk. AngularJS for instance used this, but it's a framework, so they dictate how you should be using it: One of the rules they had is that your controllers must had parameter names matching the service to be injected in, and how you should deal with minifiers, etc. As a library author, I'd never consider a change as a Breaking Change if my public API and behaviour by using the library as it should doesn't change. Anyone using Function.prototype.toString() is on their own risk.
• The example about extracting secret values, you can always move the secret values outside the code block of the function, and they won't be exposed when calling .toString()
• The point on how a function was created it's something that won't be solved with the current proposal. Currently, native functions' toString() might return "function() { [native code] }". When this proposal is implemented, unless .toString() on hidden functions also misleadingly return "[native code]", then polyfill library authors will still have to spoof .toString() to mimic native code's behaviour.
• Error stack messages as far as I know don't expose any source or parameter values. I'm not really sure what's the motivation behind hiding or even removing calls from a call stack. I think it would be misleading when debugging errors that might have been reported.

[ljharb]

It will return “native code”.