Support shared signals in Preact islands #4763

matthewp · 2022-09-14T22:38:50Z

Changes

This makes it possible to share state between Preact components rendered to islands. Signals can be created in your Astro componnents and passed to islands. They will be serialized to HTML and in the client recreated as shared signals.

Testing

Test added. Key thing about this test is checking that the keys made for the signals are the same between the islands.

Docs

N/A

changeset-bot · 2022-09-14T22:38:53Z

🦋 Changeset detected

Latest commit: 8ecb0bc

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

This PR includes changesets to release 17 packages

Name	Type
@astrojs/preact	Minor
astro	Patch
@e2e/error-cyclic	Patch
@e2e/errors	Patch
@e2e/hydration-race	Patch
@e2e/preact-component	Patch
@e2e/astro-component	Patch
@e2e/error-react-spectrum	Patch
@e2e/error-sass	Patch
@e2e/lit-component	Patch
@e2e/react-component	Patch
@e2e/solid-component	Patch
@e2e/solid-recurse	Patch
@e2e/svelte-component	Patch
@e2e/e2e-tailwindcss	Patch
@e2e/ts-resolution	Patch
@e2e/third-party-astro	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

matthewp · 2022-09-14T22:42:39Z

packages/integrations/preact/package.json

@@ -35,7 +35,8 @@
    "@babel/core": ">=7.0.0-0 <8.0.0",
    "@babel/plugin-transform-react-jsx": "^7.17.12",
    "babel-plugin-module-resolver": "^4.1.0",
-    "preact-render-to-string": "^5.2.0"
+    "preact-render-to-string": "^5.2.0",
+    "@preact/signals": "1.0.3"


This is a dep and not a peerDep so that it doesn't require users to use preact/signals.

packages/integrations/preact/src/client.ts

natemoo-re

Interesting! Looks really cool, would love to think about this a bit more before pulling the trigger.

Do you know if anyone from the Preact team might want to vet this as well?

matthewp · 2022-09-15T17:30:10Z

Sure, can pull some people in, what specific questions would you ask of them? Whether this is a good idea or something about implementation?

natemoo-re

I thought about it and I'm convinced this is a great idea! Could be super useful. Definitely fine with getting this out and we can circle back if the Preact team has any feedback.

matthewp · 2022-09-16T18:19:07Z

I'm not trying to rush this out or anything, happy to wait on it for a while to gather feedback.

I'll ping @marvinhagemeister and @developit to see if they have any thoughts on the design.

Basically the idea is to create signals in Astro components and then pass them into islands. On the client we recreate that by creating an id for each signal and then ensure that all islands get the same value.

Does this seem like a reasonable way to share state between Preact islands?
Any thoughts on the design? We are using signal.peek() to serialize the value into HTML and then using the signal in a WeakMap to generate a distinct id per signal to recreate in the client.
We have @preact/signals as a regular dependency. I was unsure if this was the right call. Putting it as a peerDependency would require everyone to use signals, which seemed wrong.

packages/integrations/preact/src/server.ts

packages/integrations/preact/package.json

marvinhagemeister · 2022-09-16T18:41:40Z

packages/integrations/preact/src/client-dev.ts

@@ -1,3 +1,4 @@
+// @ts-ignore


Q: Was the preact/debug import erroring? If we're lacking type definitions we can easily PR that to preact.

Yep, preact/debug doesn't have types at the moment: https://unpkg.com/browse/preact@10.11.0/debug/

Oh no, filed a PR. Next version will have typings for that: preactjs/preact#3732

marvinhagemeister · 2022-09-16T20:59:45Z

Basically the idea is to create signals in Astro components and then pass them into islands. On the client we recreate that by creating an id for each signal and then ensure that all islands get the same value.

Does this seem like a reasonable way to share state between Preact islands?

Yup, that's one of the exact use cases they were designed for. Since they're not bound to components it makes perfect sense to instantiate them outside of that and pass the signals to the islands directly.

Any thoughts on the design? We are using signal.peek() to serialize the value into HTML and then using the signal in a WeakMap to generate a distinct id per signal to recreate in the client.

Sounds good! You can hardly go wrong with the serialization approach. The little more tricky bit is probably restoring the state graph on the client. It's something we had in mind when building signals, but didn't have the time to explore yet. That said it should be rather straightforward for as long as you can identify each signal.

We have @preact/signals as a regular dependency. I was unsure if this was the right call. Putting it as a peerDependency would require everyone to use signals, which seemed wrong.

Good point. I think you made the best choice for Astro. Much easier when folks don't have to worry about peerDependencies.

Really excited about the things you're doing here! That's awesome stuff 👍

This reverts commit 5e46be5.

* Revert "Update preact example to match @astrojs/preact ranges (#4840)" This reverts commit d650a11. * Revert "[ci] format" This reverts commit e3c78c5. * Revert "Support shared signals in Preact islands (#4763)" This reverts commit 5e46be5.

matthewp · 2022-09-22T14:38:05Z

@marvinhagemeister we had to revert this for the time being due to preactjs/signals#197. Will release this another day though.

merlindru · 2022-09-23T18:03:55Z

preactjs/signals#197 is fixed, could we get this back pretty please?! 😄

Thank you for your work on astro @matthewp!

matthewp · 2022-09-23T20:17:11Z

Thanks for the reminder @merlindru, do you want to submit a PR that reverts #4843 and updates the library version? If not I'll get around to it eventually.

github-actions bot added pkg: astro Related to the core `astro` package (scope) pkg: example Related to an example package (scope) pkg: preact Related to Preact (scope) pkg: integration Related to any renderer integration (scope) labels Sep 14, 2022

matthewp commented Sep 14, 2022

View reviewed changes

packages/integrations/preact/src/client.ts Show resolved Hide resolved

natemoo-re reviewed Sep 15, 2022

View reviewed changes

matthewp added the semver: minor Change triggers a `minor` release label Sep 16, 2022

natemoo-re approved these changes Sep 16, 2022

View reviewed changes

matthewp commented Sep 16, 2022

View reviewed changes

packages/integrations/preact/src/server.ts Outdated Show resolved Hide resolved

marvinhagemeister reviewed Sep 16, 2022

View reviewed changes

packages/integrations/preact/package.json Outdated Show resolved Hide resolved

marvinhagemeister reviewed Sep 16, 2022

View reviewed changes

marvinhagemeister mentioned this pull request Sep 17, 2022

Add missing "types" field for preact/debug preactjs/preact#3732

Merged

matthewp added 4 commits September 21, 2022 14:40

Support signals in Preact islands

7aedf39

Add a changeset

cc7f32d

Only add signals if we need them

64053dc

Refactor signal logic into its own module

1915f23

matthewp force-pushed the preact-shared-signals2 branch from 5f5465e to 1915f23 Compare September 21, 2022 18:41

Keep track of the signals used

8ecb0bc

matthewp merged commit 5e46be5 into main Sep 21, 2022

matthewp deleted the preact-shared-signals2 branch September 21, 2022 19:21

astrobot-houston mentioned this pull request Sep 21, 2022

[ci] release #4833

Merged

matthewp added a commit that referenced this pull request Sep 22, 2022

Revert "Support shared signals in Preact islands (#4763)"

77b6145

This reverts commit 5e46be5.

matthewp mentioned this pull request Sep 22, 2022

Revert preact signals support #4843

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Support shared signals in Preact islands #4763

Support shared signals in Preact islands #4763

matthewp commented Sep 14, 2022

changeset-bot bot commented Sep 14, 2022 •

edited

matthewp Sep 14, 2022

natemoo-re left a comment

matthewp commented Sep 15, 2022

natemoo-re left a comment

matthewp commented Sep 16, 2022

marvinhagemeister Sep 16, 2022

matthewp Sep 21, 2022

marvinhagemeister Sep 22, 2022

marvinhagemeister commented Sep 16, 2022

matthewp commented Sep 22, 2022

merlindru commented Sep 23, 2022 •

edited

matthewp commented Sep 23, 2022

Support shared signals in Preact islands #4763

Support shared signals in Preact islands #4763

Conversation

matthewp commented Sep 14, 2022

Changes

Testing

Docs

changeset-bot bot commented Sep 14, 2022 • edited

🦋 Changeset detected

matthewp Sep 14, 2022

Choose a reason for hiding this comment

natemoo-re left a comment

Choose a reason for hiding this comment

matthewp commented Sep 15, 2022

natemoo-re left a comment

Choose a reason for hiding this comment

matthewp commented Sep 16, 2022

marvinhagemeister Sep 16, 2022

Choose a reason for hiding this comment

matthewp Sep 21, 2022

Choose a reason for hiding this comment

marvinhagemeister Sep 22, 2022

Choose a reason for hiding this comment

marvinhagemeister commented Sep 16, 2022

matthewp commented Sep 22, 2022

merlindru commented Sep 23, 2022 • edited

matthewp commented Sep 23, 2022

changeset-bot bot commented Sep 14, 2022 •

edited

merlindru commented Sep 23, 2022 •

edited