[BUGFIX beta] Negative indices in `positional` args proxy

[buschtoens]

In ember-link I had code that potentially accessed a negative index on a positional args proxy, which used to return undefined. Starting with the 3.23.0 beta, this throws an error.

This is because only the upper bound is checked.

[rwjblue]

Also, we need a test for this behavior (confirming that modifiers,helpers, and components can access args[-1] and get undefined back).

[buschtoens]

Cool! Let me know on the way forward regarding:

I think it may be better to change convertToInt to return null for negative indices.

I'll update the PR then and add tests. 🧪

[rwjblue]

Starting with the 3.23.0 beta, this throws an error.

Oh, also, what is the error here?

[buschtoens]

Uncaught (in promise) TypeError: ref is undefined
    valueForRef reference.js:145
    get
    normalizeLinkParams
    compute
    getValue
    ...

https://github.com/emberjs/ember.js/blob/0f3266c9b83fc9cad53b0a95f1732935d09eaf60/packages/%40ember/-internals/glimmer/lib/utils/args-proxy.ts#L104-L106

https://github.com/glimmerjs/glimmer-vm/blob/022b586405a857afdf59eebfcdae1f51428dc344/packages/%40glimmer/reference/lib/reference.ts#L153

ref is undefined as positional[-1] is undefined.

[rwjblue]

Cool! Let me know on the way forward regarding:

I think it may be better to change convertToInt to return null for negative indices.

Just chatted with @pzuraq / @krisselden about this, lets do it this way. I think that would look something like:

function convertToInt(prop: number | string | symbol): number | null {
  if (typeof prop === 'symbol') return null;

  const num = Number(prop);

  if (isNaN(num)) return null;

  return num % 1 === 0 && num > -1 ? num : null;
}

Then we just need some tests, and to update the commit message/PR title to [BUGFIX release] since 3.23 has been released.

[kategengler]

Is this still relevant?