Wrong data in `useData` after client side navigation

[pdanpdan]

Using examples/full:

• navigate to http://localhost:3000/star-wars/1 (page must be server loaded, without client navigation - so please reload page)
• navigate to 'Data fetching' using link in sidebar

An error will occur. useData will return the original object from http://localhost:3000/star-wars/1 combined with the array from http://localhost:3000/star-wars (because the base structure is an object the array content is converted to objects keys also)

I think the data should always be re-created

[brillout]

Up for a PR?

[pdanpdan]

I cannot understand where it happens

[brillout]

:+1: Let me have a look

[brillout]

I see, it's a regression introduced by https://github.com/vikejs/vike-vue/commit/a29d481dd81a8095ccb0dd5432e31a1844e35037.

Good catch.

We could delete all props before assigning the new ones.

I wanted to try whether https://github.com/vikejs/vike/discussions/1662#discussioncomment-9641666 could be a solution.

WDYT?

[pdanpdan]

I think that even if we delete all props before the shape of the original target remains. So if an object/array was made reactive it'll stay object/array

[pdanpdan]

I tried 2 approaches:

1. make dataReactive a ref(reactive(data)); set dataReactive.value = reactive(data) in changePage; make sure to return data.value in useData
2. make dataReactive a reactive({ data }); set dataReactive.data = data in changePage

For 1. we'll not get reactivity for data acquired with useData before a changePage For 2. all usage should be data.data

[brillout]

I think that even if we delete all props before the shape of the original target remains. So if an object/array was made reactive it'll stay object/array

Yes, that was actually a breaking change we introduced in 0.6.7, see CHANGELOG.md.

About 2, would it keep the current DX? In other words:

<template>
  id: {{ data.product.id }}
</template>
<script setup>
import { useData } from 'vike-vue/useData'
const data = useData()
console.log('id', data.product.id)
</script>

I like the simplicity of 1 a lot and I don't think we would need to make it reactive() anymore (I think we can and should assume data to be immutable). Although it would mean that the user would have to append .value right? In other words:

  <template>
    id: {{ data.product.id }}
  </template>
  <script setup>
  import { useData } from 'vike-vue/useData'
  const data = useData()
- console.log('id', data.product.id)
+ console.log('id', data.value.product.id)
  </script>

[pdanpdan]

1. create dataReactive as dataReactive = ref(reactive(data)); set dataReactive.value = reactive(data) in changePage; make sure to return data.value in useData

<template>
  id: {{ data.product.id }}
</template>
<script setup>
  import { useData } from 'vike-vue/useData'
  const data = useData()
  console.log('id', data.product.id)
</script>

We lose reactivity for data read with useData before changePage.

2. create dataReactive as dataReactive = reactive({ data }); set dataReactive.data = data in changePage

<template>
-  id: {{ data.product.id }}
+  id: {{ data.data.product.id }}
</template>
<script setup>
  import { useData } from 'vike-vue/useData'
  const data = useData()
-  console.log('id', data.product.id)
+  console.log('id', data.data.product.id)
</script>

A little strange when used.

3. create dataReactive as dataReactive = ref(data); set dataReactive.value = data in changePage

<template>
  id: {{ data.product.id }}
</template>
<script setup>
  import { useData } from 'vike-vue/useData'
  const data = useData()
-  console.log('id', data.product.id)
+  console.log('id', data.value.product.id)
</script>

Everything should be fine (I removed the reactive wrapper as you said above - we can keep it and I think it's exactly the same thing).

[pdanpdan]

BTW, can you think of any root level keys in pageContextReactive / pageContext that might be present when pageContextReactive is first created as reactive and missing when updating in changePage - if there are then Object.assign(pageContextReactive, pageContext) should have a cleaning stage before.

[brillout]

BTW, can you think of any root level keys in pageContextReactive / pageContext that might be present when pageContextReactive is first created as reactive and missing when updating in changePage - if there are then Object.assign(pageContextReactive, pageContext) should have a cleaning stage before.

Yes custom properties. I also think we should add a cleaning stage.

[brillout]

That's actually what I was worried about solution 2. A bit (too?) strange indeed.

So far I think ref() without reactive() is the most natural fit. In other words: I don't see why data would need to be reactive. I'd even say that making it non-reactive is a good thing as it discourages users from doing weird things with data. So, yea, the issue here is that users have to use .value outside of <template> but maybe Vue users are used to this?

1. create dataReactive as dataReactive = ref(reactive(data)); set dataReactive.value = reactive(data) in changePage; make sure to return data.value in useData

Interesting trick. From a DX perspective I guess it's the best solution?

[brillout]

I wonder if there is a way to use ref() without using reactive() in a way that preserves the "perfect DX" of solution 1.

[pdanpdan]

4. create dataReactive as dataReactive = ref(data); set dataReactive.value = data in changePage; make sure to return data.value in useData

<template>
  id: {{ data.product.id }}
</template>
<script setup>
  import { useData } from 'vike-vue/useData'
  const data = useData()
  console.log('id', data.product.id)
</script>

[!WARNING] Same caveat as for 1 - we lose reactivity for data read with useData before changePage. Is it possible that this happens (useData to be called before changePage)?

[brillout]

I'd be surprised if 4 works: AFAIK .value returns the plain value (without any wrapper) so I can't think of how Vue can track the dependency between <template> and the ref().

[pdanpdan]

It looks good to me