Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Expose unwrapStore as a low level API #6960

Merged
merged 10 commits into from
Oct 29, 2024
Merged

Conversation

GrandSchtroumpf
Copy link
Contributor

@GrandSchtroumpf GrandSchtroumpf commented Oct 10, 2024

What is it?

  • Feature / enhancement
  • Bug
  • Docs / tests / types / typos
  • Infra

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:

  • My code follows the developer guidelines of this project
  • I have performed a self-review of my own code
  • I have ran pnpm change and documented my changes
  • I have made corresponding changes to the Qwik docs
  • Added new tests to cover the fix / functionality

@GrandSchtroumpf GrandSchtroumpf requested review from a team as code owners October 10, 2024 08:12
Copy link

changeset-bot bot commented Oct 10, 2024

🦋 Changeset detected

Latest commit: 39fd36d

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

Copy link

pkg-pr-new bot commented Oct 10, 2024

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

Copy link
Contributor

github-actions bot commented Oct 10, 2024

built with Refined Cloudflare Pages Action

⚡ Cloudflare Pages Deployment

Name Status Preview Last Commit
qwik-docs ✅ Ready (View Log) Visit Preview f248246

Copy link
Member

@wmertens wmertens left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm ok with this change, but in v2 we're calling this unwrapStore which is a better name IMHO.

Can you change the export to unwrapStore?

@GrandSchtroumpf GrandSchtroumpf changed the title Expose unwrapProxy as a low level API Expose unwrapStore as a low level API Oct 14, 2024
@wmertens
Copy link
Member

@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
Copy link
Contributor Author

@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
Copy link
Contributor

shairez commented Oct 29, 2024

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

wmertens
wmertens previously approved these changes Oct 29, 2024
@shairez
Copy link
Contributor

shairez commented Oct 29, 2024

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! 🙏

@shairez shairez merged commit b466710 into QwikDev:main Oct 29, 2024
14 checks passed
@github-actions github-actions bot mentioned this pull request Oct 30, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[✨] Add the unwrapProxy as a Low-Level API
3 participants