Skip to content

Fix signals aborting context updates#302

Closed
marvinhagemeister wants to merge 5 commits into
mainfrom
context-update
Closed

Fix signals aborting context updates#302
marvinhagemeister wants to merge 5 commits into
mainfrom
context-update

Conversation

@marvinhagemeister

@marvinhagemeister marvinhagemeister commented Feb 8, 2023 •
edited
Loading

Copy link
Copy Markdown
Member

We noticed that a component that subscribes to a signal may block updates in a scenario where the signal didn't update, but context values did.

Authored together with @developit and @mdentremont in a paring session.

@changeset-bot

changeset-bot Bot commented Feb 8, 2023 •
edited
Loading

Copy link
Copy Markdown

⚠️ No Changeset found

Latest commit: 8630d1b

Merging this PR will not cause a version bump for any packages. If these changes should not result in a new version, you're good to go. If these changes should result in a version bump, you need to add a changeset.

This PR includes no changesets

When changesets are added to this PR, you'll see the packages that this PR includes changesets for and the associated semver types

Click here to learn what changesets are, and how to add one.

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

@netlify

netlify Bot commented Feb 8, 2023 •
edited
Loading

Copy link
Copy Markdown

✅ Deploy Preview for preact-signals-demo ready!

Name Link
🔨 Latest commit 8630d1b
🔍 Latest deploy log https://app.netlify.com/sites/preact-signals-demo/deploys/63e4249a1c4a8e0008161289
😎 Deploy Preview https://deploy-preview-302--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 settings.

@github-actions

github-actions Bot commented Feb 8, 2023 •
edited
Loading

Copy link
Copy Markdown
Contributor

Size Change: +140 B (0%)

Total Size: 68.5 kB

Filename Size Change
docs/dist/assets/index.********.js 836 B -243 B (-23%) 🎉
docs/dist/assets/signals.module.********.js 2.1 kB +130 B (+7%) 🔍
docs/dist/demos-********.js 3.35 kB -1 B (0%)
packages/preact/dist/signals.js 1.33 kB +129 B (+11%) ⚠️
packages/preact/dist/signals.mjs 1.27 kB +125 B (+11%) ⚠️
ℹ️ View Unchanged
Filename Size
docs/dist/assets/client.********.js 46.4 kB
docs/dist/assets/jsxRuntime.module.********.js 282 B
docs/dist/assets/preact.module.********.js 4 kB
docs/dist/assets/signals-core.module.********.js 1.36 kB
docs/dist/assets/style.********.js 21 B
docs/dist/assets/style.********.css 1.21 kB
docs/dist/basic-********.js 244 B
docs/dist/nesting-********.js 1.13 kB
docs/dist/react-********.js 239 B
packages/core/dist/signals-core.js 1.42 kB
packages/core/dist/signals-core.mjs 1.44 kB
packages/react/dist/signals.js 1 kB
packages/react/dist/signals.mjs 919 B

compressed-size-action

@marvinhagemeister @developit
Fix signals aborting context updates
5f6f7c7 
Co-authored-by: Jason Miller <jason@developit.ca>
@marvinhagemeister

marvinhagemeister commented Feb 8, 2023

Copy link
Copy Markdown
Member Author

Managed to narrow the remaining issue to a reproducible test case.

@marvinhagemeister marvinhagemeister mentioned this pull request Feb 9, 2023
Merged
@marvinhagemeister

marvinhagemeister commented Feb 9, 2023

Copy link
Copy Markdown
Member Author

Closing as the actual source of the problem was a bug in Preact. See preactjs/preact#3889 .

@marvinhagemeister marvinhagemeister deleted the context-update branch February 10, 2023 09:50
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

No reviews

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

1 participant

@marvinhagemeister