Detect and unwrap signal properties

[JoviDeCroock]

Fixes #422

[changeset-bot]

🦋 Changeset detected

Latest commit: e7d241c

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

This PR includes changesets to release 1 package

Name	Type
preact-render-to-string	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

[marvinhagemeister]

Wondering if we should use duck-typing here like https://github.com/denoland/fresh/blob/e65643c482de9b5afee5d52af3ccc88941c35b5c/src/runtime/server/preact_hooks.tsx#L355-L362 . That way we can keep rts dependency free and don't have to keep it in sync with major @preact/signals version increases.

[JoviDeCroock]

This might end up being a much larger perf hit in the critical path though

[marvinhagemeister]

Not sure if x !== null && typeof x === "object" is more or less expensive than an instanceof check. I let you decide on that one.

[rschristian]

I'd rather avoid adding a dep for a singular instanceof check myself, and at least as of a few years ago, duck-typing with 1-2 properties usually outperformed instanceof in Chrome. No idea of what fully happens internally there, but naively, makes sense to me.

If you've measured it & it is a perf issue, fair enough of course. I'll have some time in a bit to check the benches (if you haven't already).

Edit: At least on my system, the check Marvin linked to above doesn't seem to perform any worse than instanceof. Ran the benches a few times and there wasn't any noticeable drop.

[marvinhagemeister]

LGTM

[hronro]

It's great to see this has been merged! Are there any plans to release a new version that includes this fix?
@JoviDeCroock @marvinhagemeister

[rschristian]

We'll release it when we're ready, please be patient.

You can use patch-package or pnpm patch in the meantime if you need it, should be pretty straightforward to add.

[rschristian]

Looks like this was accidentally left around, will need to be removed but I gotta get to bed -- I can do it tomorrow but wanted to drop a mention at least so we don't release this.

[hronro]

We'll release it when we're ready, please be patient.

You can use patch-package or pnpm patch in the meantime if you need it, should be pretty straightforward to add.

Thanks for your reply. I'm using Deno with the Fresh framework, which isn't easy to patch a npm dependency, neither pactch-package nor pnpm patch works for me. Anyway, I appreciate the work you guys have done and hope there could be a new release soon.