Fix complex property globs

[edemaine]

The following examples weren't working before:

• <Component {f()?.{nx,ny}}> (ref caused trouble — I encountered this in the real world which is what sent me down this path)
• {x().{a,b}, y()?.{c,d}} (didn't properly merge with multiple refs)

Also improved globs to not use refs if we only need one property, which occurs in a bunch of dynamic import examples.

[STRd6]

A good improvement!

I added some notes that could be addressed in a follow-up about better resolution of ?. chaining before the glob and exactly what we want the behavior to be in those cases.

[STRd6]

We probably want to preserve the order here if possible. I believe it can have an impact if the same key also appears in the spread.

[edemaine]

Agreed. This is an issue "inherited" from my old JSX attribute code, which e.g. compiles <Foo {a, [b]: x, c}> to <Foo a={a} c={c} {...{ [b]: x }} />. But it shouldn't be too hard to fix, by accumulating maximal chunks that need {...{ treatment. I'll leave it for a future PR for easier review.

[STRd6]

This may be out of scope for this particular fix but if x.y() ends up undefined then .a and .b on the ref will throw instead of being short circuited.

Also related to that we should decide what to return in the case of the ref not resolving. I think undefined is correct but in the previous example with the ?. directly before the glob we're returing {nx: undefined, ny: undefined}

I think the check will end up being something like ((ref = x.y()) != null ? ((ref=ref.z), { a:ref.a, b:ref.b }) : ref

Sorry it's dense but note the use of the comma operator inside the first ternary branch to manually update the ref. We may need to do it for each time ?. appears before the glob.

Essentially manually short circuiting every ?. along the chain.

[edemaine]

Hmm, good point: this is probably unexpected behavior that x?.y.{a,b} crashes when x is undefined. The behavior is not changed by this PR, but this PR's compilation does make it potentially more difficult

One challenge with your compilation is that globs can appear inside object expressions, e.g. {c, x?.y.{a,b}}. That's why I switched to setting the ref here to get set in the first element instead of outside in a parenthesized expression; the latter wasn't properly mixable. But of course it's possible to combine the two strategies as you say.

A simpler workaround, but with different behavior, would be to compile x()?.y.z.{a,b} into {a: (ref = x()?.y.z)?.a, b: ref?.b}. In other words, add a ? after ref if there was any ? earlier in the chain. But this would return {a: undefined, b: undefined} instead of undefined. I'm not totally sure which behavior is better... I could see preferring {a: undefined, b: undefined} if e.g. doing the following:

{a, b} := object.{a,b}

Although, in that case, we could just write {a, b} := object. Maybe a better example is returning a projected object? But again it's unclear which is better/more intuitive: returning undefined or an object containing undefineds... What do you think?

[STRd6]

I think ultimately we'll need to short circuit at exactly each ?. The ones outside the glob prevent adding any keys, the ones inside the glob add the key but its value may be undefined.

Checking later in aggregate wouldn't quite be semantically correct. In this case x?.y when x is defined but y is undefined should throw.

Spacing for clarity:

{c, x?.y.{a,b}}
---
let ref;
({ 
  c, 
  ...( (ref = x) != null && (    // short circuit at `x?.`
    ref = ref.y,    // update ref
    {a: ref.a, b: ref.b}    // object for spread
  ) )
})

For the case of projecting I think we'd want to explicitly show that we'd prefer to ignore the missing object rather than to absorb the error. TS will also warn us if the value may potentially be undefined.

{a, b} := {...object?.{a,b}}

Though a case could be made that the ?. immediately preceding the glob {...} may prefer constructing the projection anyway.

[edemaine]

Interesting. The {...glob} splat perspective is clarifying: if the glob is undefined (because of an earlier failure), it doesn't add anything to the object. I guess this is somewhat in line with our recent object comprehensions too (which are kinda like implicit ... splats).

I'm still not totally sure which interpretation is correct, but I'm coming around to and you're probably right that undefined is more natural behavior than {a: undefined, b: undefined}.

Note that if we want the latter, we could avoid ... I think. (I do agree that we need to check each possible failure separately though; my ?. attempt is indeed broken.)

{c, x()?.y.{a,b}}
---
let xref, yref;
({
  c,
  a: (xref = x()) == null ? undefined :
    (yref = xref.y).a,
  b: xref == null ? undefined : yref.b
})