fix: cleanup old keys and use shallowReactive instead of reactive for dataReactive and pageContextReactive #116, #121

[pdanpdan]

BREAKING CHANGE: data from +data hook must be a non-array object (if you need to return a list wrap it in an object)
BREAKING CHANGE: both useData and usePageContext return shallowReactive instead of reactive

closes #116, closes #121

[pdanpdan]

A fix for pageContextReactive is still needed (in changePage)

[brillout]

I think I found a use case where this unfortunately doesn't work. Let me try to reproduce on /examples/full/.

[pdanpdan]

BTW, if we want to treat data as readonly we could go with shallowRef
I'll try after we check possible breaking case(s)

[brillout]

984ef60 breaks this PR.

To reproduce:

1. cd examples/full/
2. pnpm run dev
3. localhost:3000/hello

   Observe text: "Hi anonymous."

4. Click on /hello/eli.

   Observe text is still: Hi anonymous..
   (It should be Hi eli..)

5. Refresh page.

   Observe text is now: Hi eli..

[pdanpdan]

It looks like we need to return the full ref, so the DX changes in JS/TS (stays the same in template)

[brillout]

in order to keep reactivity we need to expose the shallowref

Maybe a hack would be:

const dataRef = ref(data)
Object.assign(dataRef, data)

Not sure if it's worth it. I don't know whether data.value.product.id is a weird DX for Vue users.

[pdanpdan]

I don't know whether data.value.product.id is a weird DX for Vue users.

Not weird DX, especially if editor interprets TS types you get full autocomplete

Maybe a hack would be: ...

I'm not sure where would that fit

[brillout]

Ok 👍

On my side I think we should go for this shallowRef() solution then.

Thoughts on pageContext? In principle, it should also just be a shallowRef() I think. But what about the DX 🤔

[pdanpdan]

I'll clean up this PR and let's move to another issue for pageContext

[brillout]

👍 And let's also decide about pageContext before merging. Probably best to holistically decide for both.

[pdanpdan]

Cleanup done here and opened sibling issue #121

[pdanpdan]

This should be now ready

[brillout]

I made some minor changes, let me know if you disagree with something. Otherwise I'll squash, merge, and release.

[pdanpdan]

LGTM

[brillout]

Merged 🎉

Thank you the neat collaboration 💚

[brillout]

And released as 0.7.2 🚀

@4350pChris I went ahead without your input because this PR fixes a couple of bugs without introducing any (conceptual) breaking change. But I'm more than happy to have the conversation whether we should go for shallowRef() instead of shallowReactive() (or something entirely else) with you — in case you disagree with some of the discussions we had at #116 and #121.