Skip to content

Add watched and unwatched #634

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

Merged
merged 9 commits into from
May 28, 2025
Merged

Add watched and unwatched #634

merged 9 commits into from
May 28, 2025

Conversation

JoviDeCroock
Copy link
Member

@JoviDeCroock JoviDeCroock commented Dec 27, 2024

Resolves #351
Resolves #428

This adds a second argument to signal which are the options, currently this adds two properties, watched and unwatched. These will most likely serve third party libraries a lot more than first party consumers.

  • watched will trigger when the first subscriber subscribes to a signal
  • unwatched will trigger when the last subscriber unsubscribes from a signal

The second parameter can in the future be used for other RFC's in our pipeline like the equality function. The addition of this parameter is in line with the TC39 proposal for browser-native signals https://github.com/tc39/proposal-signals

I opted not to add this to computed for the time being as the signal itself is the atom while computed is more of a derived value. Not sure if a utility like this would be useful there. In the TC39 proposal they also add this to computed, so might as well for parity.

Note that calling the watcher on the subscribe is slightly distinct from how we normally treat signals, i.e. we calculate the computed when its value is accessed for the first time.

Copy link

changeset-bot bot commented Dec 27, 2024

🦋 Changeset detected

Latest commit: 9dd86cf

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

This PR includes changesets to release 3 packages
Name Type
@preact/signals-core Minor
@preact/signals Minor
@preact/signals-react 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

netlify bot commented Dec 27, 2024

Deploy Preview for preact-signals-demo ready!

Name Link
🔨 Latest commit 9dd86cf
🔍 Latest deploy log https://app.netlify.com/projects/preact-signals-demo/deploys/6836fcc1f929d1000830419d
😎 Deploy Preview https://deploy-preview-634--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 project configuration.

Copy link
Contributor

github-actions bot commented Dec 27, 2024

Size Change: +253 B (+0.3%)

Total Size: 84.5 kB

Filename Size Change
docs/dist/assets/bench.********.js 1.59 kB -1 B (-0.06%)
docs/dist/assets/client.********.js 46.2 kB +2 B (0%)
docs/dist/assets/index.********.js 1.09 kB -1 B (-0.09%)
docs/dist/assets/signals-core.module.********.js 1.5 kB +90 B (+6.36%) 🔍
docs/dist/assets/signals.module.********.js 2.04 kB +1 B (+0.05%)
docs/dist/demos-********.js 4.32 kB -2 B (-0.05%)
docs/dist/nesting-********.js 1.13 kB -2 B (-0.18%)
docs/dist/react-********.js 239 B -3 B (-1.24%)
packages/core/dist/signals-core.js 1.53 kB +85 B (+5.87%) 🔍
packages/core/dist/signals-core.mjs 1.55 kB +74 B (+5.02%) 🔍
packages/preact/dist/signals.js 1.57 kB +3 B (+0.19%)
packages/preact/dist/signals.mjs 1.53 kB +7 B (+0.46%)
ℹ️ View Unchanged
Filename Size
docs/dist/assets/jsxRuntime.module.********.js 283 B
docs/dist/assets/preact.module.********.js 4.01 kB
docs/dist/assets/style.********.js 21 B
docs/dist/assets/style.********.css 1.24 kB
docs/dist/basic-********.js 243 B
packages/react-transform/dist/signals-*********.js 4.9 kB
packages/react-transform/dist/signals-transform.mjs 4.13 kB
packages/react-transform/dist/signals-transform.umd.js 5.01 kB
packages/react/dist/signals.js 188 B
packages/react/dist/signals.mjs 150 B

compressed-size-action

@andrewiggins
Copy link
Member

Is this mostly to align with the signal proposal? If so, perhaps we can wait for that proposal to go to Stage 2 or 3 and we have more confidence in the API landing before we implement (just to avoid any churn).

if we have some use cases we'd like to use this for now, it does seems reasonable to me.

@mxck
Copy link

mxck commented Jan 7, 2025

Is this mostly to align with the signal proposal? If so, perhaps we can wait for that proposal to go to Stage 2 or 3 and we have more confidence in the API landing before we implement (just to avoid any churn).

if we have some use cases we'd like to use this for now, it does seems reasonable to me.

Hi! I’m one of the people who requested this feature.

The main use case is for subscriptions over WebSocket or data polling using interval/timeout, but only when the signal is mounted/used.

I previously asked about how we could achieve a setup where data updates occur only if the signal is actively in use. Here’s the link to my question in discussions: link.

Copy link
Member

@marvinhagemeister marvinhagemeister left a comment

Choose a reason for hiding this comment

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

LGTM

@JoviDeCroock JoviDeCroock merged commit 62bed44 into main May 28, 2025
6 checks passed
@JoviDeCroock JoviDeCroock deleted the watched branch May 28, 2025 12:12
@github-actions github-actions bot mentioned this pull request May 28, 2025
Comment on lines -30 to -36
- [Preact Integration](./packages/preact/README.md#preact-integration)
- [Hooks](./packages/preact/README.md#hooks)
- [Rendering optimizations](./packages/preact/README.md#rendering-optimizations)
- [Attribute optimization (experimental)](./packages/preact/README.md#attribute-optimization-experimental)
- [React Integration](./packages/react/README.md#react-integration)
- [Hooks](./packages/react/README.md#hooks)
- [Rendering optimizations](./packages/react/README.md#rendering-optimizations)
Copy link
Member

@rschristian rschristian Jun 3, 2025

Choose a reason for hiding this comment

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

Only noticed now, but what was the reason for removing this? It was pretty convenient and directed users to the correct place for their specific use (even when published, NPM will link to the correct directory back here).

Especially problematic for React users, of which we've had so many who thought they could just install & match the APIs listed below.

Edit: Maybe it'd be better to finally split the repo root ReadMe from the core ReadMe. Will take a look at that.

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.

Adding a callback that will be triggered when the signal starts to be observed Subscription integration for third-party libraries
5 participants