QwikDev / qwik

Instant-loading web apps, without effort
https://qwik.dev
MIT License
20.85k stars 1.31k forks source link

Expose unwrapStore as a low level API #6960

Closed GrandSchtroumpf closed 3 weeks ago

GrandSchtroumpf commented 1 month ago

What is it?

Description

unwrapProxy was marked as a @public API in JSDoc but wasn't exposed to the user. Exposing unwrapProxy enables developer to clone the content of a useStore() using structureClone or IndexedDB

fix #6135

Checklist:

changeset-bot[bot] commented 1 month ago

🦋 Changeset detected

Latest commit: 39fd36ddef591c46b7718775535c6f45e522c538

The changes in this PR will be included in the next version bump.

This PR includes changesets to release 4 packages | Name | Type | | --------------------- | ----- | | @builder.io/qwik | Minor | | eslint-plugin-qwik | Minor | | @builder.io/qwik-city | Minor | | create-qwik | Minor |

Not sure what this means? Click here to learn what changesets are.

Click here if you're a maintainer who wants to add another changeset to this PR

pkg-pr-new[bot] commented 1 month ago

Open in Stackblitz

npm i https://pkg.pr.new/@builder.io/qwik@6960
npm i https://pkg.pr.new/@builder.io/qwik-city@6960
npm i https://pkg.pr.new/eslint-plugin-qwik@6960
npm i https://pkg.pr.new/create-qwik@6960

commit: f248246

github-actions[bot] commented 1 month ago
built with Refined Cloudflare Pages Action

⚡ Cloudflare Pages Deployment

Name Status Preview Last Commit
qwik-docs ✅ Ready (View Log) Visit Preview f248246af4b2ba3c19e38deb3518e8eaa3d2b506
wmertens commented 3 weeks ago

@GrandSchtroumpf can you update the change set and API docs so they all say unwrapStore?

@shairez shall we put this in 1.9.2?

GrandSchtroumpf commented 3 weeks ago

@wmertens I updated the changeset. The doc still refers to unwrapProxy because this is the original name of the function. I can change unwrapProxy to unwrapStore everywhere, but I don't want to create conflict with v2.

shairez commented 3 weeks ago

Thanks @GrandSchtroumpf !

Add a few short comments about the changesets After that we can merge

@wmertens please resolve the "request changes" when you have a chance

shairez commented 3 weeks ago

nvm @GrandSchtroumpf I did the modifications on the changeset file because I wanted to merge asap to make this part of 1.9.2

But please read my review just as FYI for next time, thanks again for the PR! 🙏