preactjs / signals

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

fix(react-signals-transform): Support nested functions accessing signals #582

Closed JoviDeCroock closed 4 months ago

JoviDeCroock commented 4 months ago

This was a bit more complex than initially anticipated due to how we have chosen to transform, I wanted to make as little change as possible so here goes...

Fixes https://github.com/preactjs/signals/issues/552 Supersedes https://github.com/preactjs/signals/pull/553

changeset-bot[bot] commented 4 months ago

đŸĻ‹ Changeset detected

Latest commit: 83bcaeb7154802624c21e4e51a2a0f3f8e27eb0a

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-transform | 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 4 months ago

Deploy Preview for preact-signals-demo ready!

Name Link
Latest commit 83bcaeb7154802624c21e4e51a2a0f3f8e27eb0a
Latest deploy log https://app.netlify.com/sites/preact-signals-demo/deploys/6687981c039d200008fea472
Deploy Preview https://deploy-preview-582--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.

github-actions[bot] commented 4 months ago

Size Change: +491 B (+0.61%)

Total Size: 81 kB

Filename Size Change
packages/react-transform/dist/signals-*********.js 4.91 kB +165 B (+3.48%)
packages/react-transform/dist/signals-transform.mjs 4.17 kB +161 B (+4.02%)
packages/react-transform/dist/signals-transform.umd.js 5.03 kB +165 B (+3.39%)
ℹī¸ View Unchanged | Filename | Size | | :--- | :---: | | `docs/dist/assets/client.********.js` | 46.3 kB | | `docs/dist/assets/index.********.js` | 838 B | | `docs/dist/assets/jsxRuntime.module.********.js` | 284 B | | `docs/dist/assets/preact.module.********.js` | 4.03 kB | | `docs/dist/assets/signals-core.module.********.js` | 1.4 kB | | `docs/dist/assets/signals.module.********.js` | 2.02 kB | | `docs/dist/assets/style.********.js` | 21 B | | `docs/dist/assets/style.********.css` | 1.21 kB | | `docs/dist/basic-********.js` | 244 B | | `docs/dist/demos-********.js` | 3.41 kB | | `docs/dist/nesting-********.js` | 1.13 kB | | `docs/dist/react-********.js` | 242 B | | `packages/core/dist/signals-core.js` | 1.45 kB | | `packages/core/dist/signals-core.mjs` | 1.47 kB | | `packages/preact/dist/signals.js` | 1.27 kB | | `packages/preact/dist/signals.mjs` | 1.22 kB | | `packages/react/dist/signals.js` | 188 B | | `packages/react/dist/signals.mjs` | 150 B |

compressed-size-action

JoviDeCroock commented 4 months ago

Apparently I missed the Suspense tests, I would probably also need to look at higher order components before feeling confident as we might need an early return case for hoc's.

Should be covered in https://github.com/preactjs/signals/pull/582/commits/a8a4f8685a8c0e43fd39d1d15a1c6dc5b354acd2 all tests are passing for me, let's see about CI