preactjs / signals

Manage state with style in every framework
https://preactjs.com/blog/introducing-signals/
MIT License
3.64k stars 89 forks source link

Fix `React.lazy` and `React.forwardRef` component updates tracking. #427

Open XantreDev opened 9 months ago

XantreDev commented 9 months ago

Based on @ywang1724 pr #342. Appreciate for help.

Added tests of reactiveness for React.lazy and React.forwardRef and made program to pass it. Updated pnpm-lock while installation, because ci failed

changeset-bot[bot] commented 9 months ago

🦋 Changeset detected

Latest commit: cf51a22f267b9dde299ebd3c4c48ad64fc89f15b

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

This PR includes changesets to release 1 package | Name | Type | | --------------------- | ----- | | @preact/signals-react | Patch |

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

netlify[bot] commented 9 months ago

Deploy Preview for preact-signals-demo ready!

Name Link
Latest commit ae15b16d2e1d0dbde05f28b6045d40050158bd76
Latest deploy log https://app.netlify.com/sites/preact-signals-demo/deploys/652a4c58d3b2880008214fd4
Deploy Preview https://deploy-preview-427--preact-signals-demo.netlify.app
Preview on mobile
Toggle QR Code...

QR Code

Use your smartphone camera to open QR code link.

To edit notification comments on pull requests, go to your Netlify site configuration.

XantreDev commented 9 months ago

@marvinhagemeister @JoviDeCroock Review me please). Its fix for 1.2.1 version, because its most stable one and for now the only one working in react native.

301 #420

rschristian commented 9 months ago

Its fix for 1.2.1 version, because its most stable one and for now the only one working in react native.

Am I understanding you correctly in that this is based on 1.2.1, instead of the branch head? Definitely can't merge this into main if so.

Precisely, is it 1.2.1 or 1.2.2? Should probably be the latter and we can make a branch for you to merge into if needs be.

XantreDev commented 9 months ago

I think it should be in another branch, this changes for 1.2.2

XantreDev commented 9 months ago

I think we can cherry pick commit related with adding tests for forwardRef and lazy. But other things merge into separate branch.

XantreDev commented 8 months ago

@JoviDeCroock @rschristian what should i do to release 1.2.3 with this fixes? I want to fix this bug, because it causes problems in my project

rschristian commented 8 months ago

Sorry for the delay, just been incredibly busy and missed this. I created a new branch from 1.2.2 and switched this to target it. You might want to take a look at the pnpm.lock file though, that diff looks odd and unnecessary. Edit: Just reread prior comments -- revert the change entirely. If it's needed for the CI or the CI needs alterations, that can be done in another PR.

I don't use React and therefore this package, so I can't really comment here. @andrewiggins is probably the best person to comment on this (if he has the time)

XantreDev commented 8 months ago

431 Extracted lockfile changes to separate PR

rschristian commented 8 months ago

The CI being limited to PRs against main is one way to fix the issue 😂

aspizu commented 6 months ago

any updates on this?

XantreDev commented 6 months ago

any updates on this?

You can use the 2.0.0 version with the babel plugin now