Computed props with rest arguments get... weird

[Rich-Harris]

Discovered during #1540, opening a separate issue because I'm not entirely sure what to do with it.

If you have a situation like this (won't work in the REPL, because object rest support is waiting on #1542)...

<pre>{JSON.stringify(props)}</pre>

<script>
  export default {
    data: () => ({
      unwanted: 1,
      wanted: 2
    }),

    helpers: {
      JSON
    },

    computed: {
      props: ({ unwanted, ...props }) => props
    }
  };
</script>

...then it will initially render this:

<pre>{"wanted":2}</pre>

But if wanted changes to 3, you get this:

<pre>{"wanted":3,"props":{"wanted":2}}</pre>

And then this:

<pre>{"wanted":4,"props":{"wanted":3,"props":{"wanted":2}}}</pre>

Somehow, we need to exclude props from the object that props is based on. But we can't do that like this...

computed: {
  props: ({ unwanted, props, ...everythingElse }) => everythingElse
}

...because Svelte will see that as props depending on itself (at least, it will post-#1543), which is an error.

Banning object rest in computed property arguments isn't a solution, because people can just do this instead:

computed: {
  props: (state) => {
    const { unwanted, ...props } = state;
    return props;
  }
}

Maybe in cases where a computed property depends on the entire state object (either explicitly, or because its argument has a rest element) we remove the property itself? So in the case above,

state.props = props(removeProp(state, 'props'));

// later
function removeProp(obj, prop) {
  const clone = {};
  for (const k in obj) {
    if (k !== prop) clone[k] = obj[k];
  }
  return clone;
}

It feels like we're getting into some weird territory though, and it wouldn't solve this case, in which b.b.b.b.b will eventually equal b:

computed: {
  a: ({ unwanted, ...rest }) => rest,
  b: ({ a }) => a
}

[TehShrike]

This doesn't solve any of the real issues, but my personal choice for the clone-without-this-property pattern is

const clone = Object.assign({}, obj)
delete clone[k]

even though it's still not great.

---

It's true there are weird territories you could get into, but personally something like your a -> b computed property example looks fishy enough that it wouldn't surprise me if there were weird effects, whereas having props show up in props: ({ unwanted, ...rest }) => rest threw us for a hard loop.

¯\_(ツ)_/¯

[Conduitry]

This was also taken care of in #1628 I think.

[fjorgemota]

This same issue happens if you pass the entire state object without using object rest. See in REPL. (try changing wanted value..)

Is this an expected behavior? (or should I create an new issue just for that?)

[fjorgemota]

ping @Rich-Harris and @Conduitry (sorry for the ping, just wanna to confirm this and the issue seems related)

[Conduitry]

No big opinions on whether that belongs to this issue or it should be another one - but it does seem like that's a bug though. If we're going to automatically exclude the current computed property in object-rest situations, it's probably a good idea to do so in full-state situations. Perhaps here should always use exclude, regardless of whether hasRestParam is set.

[Rich-Harris]

Fixed in 2.11 — thanks