Ember.js: [Bug] Hash helper dynamically constructs new object each time it is accessed from let helper

[elwayman02]

🐞 Describe the Bug

When using the hash helper in a template, the result of that helper does not appear to get reused in all situations where it is consumed. In the flow of the template, we appear to call the hash helper once and then there appears to be lazy access to the hash helper each time its result is used. This results in the hash being recreated rather than using the same hash.

🔬 Minimal Reproduction

I tested this issue using Ember's guidFor to try different patterns and see if guidFor consistently returned the same guid as expected:

https://ember-twiddle.com/93e426e0696a9ce7a3f20b2a0ceecfbe?openFiles=templates.application%5C.hbs%2Ctemplates.components.double-conditional-yield%5C.hbs

😕 Actual Behavior

As you can see in the reproduction, there are several cases where the result of the hash helper changes further down the DOM tree, even though the flow of the template dictated just a single call to it.

🤔 Expected Behavior

guidFor should be able to return the same result for any object generated by a single call to the hash helper

🌍 Environment

• Ember: 3.18
• Node.js/npm: 12
• OS: MacOS
• Browser: Chrome

➕ Additional Context

I believe this issues with the hash helper extends beyond just its use in combination with the let helper, but I have not yet been able to reproduce the exact sequence in a Twiddle.

[rwjblue]

https://ember-twiddle.com/5b71d84fc6f11e9aa9a6d9f1a5a625e7?openFiles=templates.application%5C.hbs%2C is a bit smaller/easier of a reproduction (less moving parts), basically:

{{#let (hash foo='bar') as |foo|}}
  {{guid-for foo}}
  {{guid-for foo}}
{{/let}}

Given the implementation of a guid-for helper that essentially just calls Ember.guidFor, results in two different object instances for the foo block param.

In other words, the (hash value is not stable and is being recreated for each access.

[chancancode]

We don't really have any guarantees for when/if or how many times a helper is invoked, and even if we make the case in @rwjblue's reduced reproduction work, it will still result in a different instance when any of the arguments passed to the helper "changes" (according to the tags, but they could have the same === value).

[elwayman02]

@chancancode I think it's reasonable to expect a recomputation to take place when a change is detected, even if the === value is the same. I'm more concerned about the fact that the flow of the template code does not reflect what's actually happening. It appears as if you are generating an object and then reusing it several times, when you are actually generating many new instances of the object instead. I would consider that a bug and unexpected behavior.

[chancancode]

I think that metal model isn't quite correct, or at least it doesn't match how we intended things to work. {{#let}} is more or less just an alias, it has roughly the same semantics as inlining the expression in an AST transform. On the other hand, the runtime should be allowed to make optimizations such that, if the helper is pure, and the arguments are the same it can be invoked as little as one time (with the result "reused" in all identical call-sites), or as many as times as the implementation requires, without any particular timing or ordering guarantee.

[elwayman02]

Describing it as an alias implies that it's only evaluated once, no matter how many times it's accessed, imo.

[chancancode]

I disagree with that definition of the word alias 😄 This, or this, was my intended meaning of alias. You _alias_ a memory location and access it via the alias, but how many times and the timing of the underlying memory access is an implementation detail that the compiler, runtime and hardware is allowed to optimize freely.

Anyway, sorry for being pointlessly argumentative for no reason. If the analogy was not helpful I am happy to pick a different one. And I am not saying you are somehow wrong to be surprised – if you are surprised, then you are surprised. I am just trying to explain that there is an alternative mental model – one which the design/implementation is based on – where this wouldn't be surprising.

Surprises aside, there are good performance and implementation reasons for favoring the current semantics or at least keeping things open-ended here. And while I understand you want this specific case "fixed" – and I agree there are relatively simple/easy way we can use to "fix" it, that would make this case work.

However, so far I am not in favor or fixing, both because I would prefer to leave open the possibility for future implementation changes/optimizations, but also I think the fix would only make things work in this very specific case, and as I explained above, there exists plenty of other cases (that we can't as easily "fix") that would still feel "broken" under that mental model, and perhaps would feel "more broken" if this special case is "fixed".

Perhaps you can elaborate and explain the actual real world use case/pattern that you are trying to achieve with this? It will either help me understand why this is important/useful/common enough to override my other concerns, or perhaps it will help me point out to you why the fix you won't would not work in a slightly tweaked version of your example, and suggest alternative approaches.

[elwayman02]

In computer programming, aliasing refers to the situation where the same memory location can be accessed using different names.

How does this not imply that you're going to receive the same reference no matter where you access it from? If anything, I would argue that the behavior we're seeing here is explicitly NOT an alias - It is in fact giving you new objects that exist at different memory locations and are copies of each other, not the same instance. So I'm very confused as to why you disagreed with my definition of the word alias and then immediately linked to a source that agrees with my definition.

[elwayman02]

My real-world use case wherein I encountered this bug is more or less a more elaborate implementation of the DoubleYield component in the repro example. We need to ensure that any argument passed in to the two yielded components has the same reference because we are generating ID from them using GuidFor in order to have the parent component (in this example, DoubleYield) be able to pair components that received the same arguments via a data attribute containing the results of the GuidFor call. If there is the possibility that a single call to a yielded component can result in two distinct instances of the argument object simply because we invoke the yield twice internally (or more instances, as you can see in the toggle example at the bottom of my twiddle - Click the button a bunch to watch those guids soar!), then we can no longer deterministically generate IDs based on these arguments.

[pzuraq]

@chancancode I feel like the expectations from a language design perspective for users are that the value referenced in a scope will generally be static for a single pass through that scope. I can't think of a programming language that _does_ change the value of a binding _during_ a function call, without explicitly being done by the user.

I really think this behavior is very confusing as is, and I don't think that on a surface level, it can be seen as correct. If you understand how helpers work, and how the system flows as a whole, yes, you could come to that conclusion, but someone just learning the language will likely be very confused, and that's not a great outcome.

While I think the ability to have optimizations is important, I don't think we should sacrifice logical correctness for performance, and I think based on language design precedence and general programming language norms, this does sacrifice logical correctness. We should discuss more though, definitely, and see if there's something we can do here.