4 ms·

The implementation involves unsafe monkey-patching: 1. For React, a hook is injected into every single component (!) regardless of whether it uses signals or n

The implementation involves unsafe monkey-patching: 1. For React, a hook is injected into every single component (!) regardless of whether it uses signals or not via __SECRET_INTERNALS_DO_NOT_USE_OR_YOU_WILL_BE_FIRED. 2. For React, React.createElement is patched so that it can render signal values as Text nodes. 3. For Preact, they claim it's using a "pluggable renderer", yet they monkey-patch the global shouldComponentUpdate (and they don't call the old one so they break other patchy libraries) [1] - https://github.com/preactjs/signals/blob/d25e8bac09c94ed3bad7f12fe380becd8bf1a7ad/packages/react/src/index.ts#L131 https://github.com/preactjs/signals/blob/d25e8bac09c94ed3bad... [2] - https://github.com/preactjs/signals/blob/d25e8bac09c94ed3bad7f12fe380becd8bf1a7ad/packages/react/src/index.ts#L27 https://github.com/preactjs/signals/blob/d25e8bac09c94ed3bad... [3] - https://github.com/preactjs/signals/blob/d25e8bac09c94ed3bad7f12fe380becd8bf1a7ad/packages/preact/src/index.ts#L227 https://github.com/preactjs/signals/blob/d25e8bac09c94ed3bad...

pyentropy 4y ago
Update: it completely breaks with React 18 and strict mode enabled - https://github.com/preactjs/signals/issues/70 https://github.com/preactjs/signals/issues/70
- bennettdams 4y ago
  Does someone know why the counter for this issue is at "70", but the repo only shows "21" issues in total? Maybe there are deleted issues that are not part of the total counter? https://github.com/preactjs/signals/issues?q=is%3Aissue https://github.com/preactjs/signals/issues?q=is%3Aissue
  - mhagemeister 4y ago
    No, that's because GitHub uses the same number for both issues and pull requests. We did about 70 pull requests. You can verify that yourself by going on an issue and incrementing/decrementing the number until you hit a PR at which point you'll be redirected.
skybrian 4y ago
For Preact, there is a TODO pointing to [1] which was merged in 10.10.3, so hopefully that can be fixed soon? [1] https://github.com/preactjs/preact/pull/3671 https://github.com/preactjs/preact/pull/3671
lioeters 4y ago
Good catch. If this were an internal library, it wouldn't have passed code review.
newbieuser 4y ago
the best vocabulary to describe the monkey patching react ecosystem

pyentropy 4y ago

Update: it completely breaks with React 18 and strict mode enabled - https://github.com/preactjs/signals/issues/70 https://github.com/preactjs/signals/issues/70

bennettdams 4y ago
Does someone know why the counter for this issue is at "70", but the repo only shows "21" issues in total? Maybe there are deleted issues that are not part of the total counter? https://github.com/preactjs/signals/issues?q=is%3Aissue https://github.com/preactjs/signals/issues?q=is%3Aissue
- mhagemeister 4y ago
  No, that's because GitHub uses the same number for both issues and pull requests. We did about 70 pull requests. You can verify that yourself by going on an issue and incrementing/decrementing the number until you hit a PR at which point you'll be redirected.

skybrian 4y ago

For Preact, there is a TODO pointing to [1] which was merged in 10.10.3, so hopefully that can be fixed soon? [1] https://github.com/preactjs/preact/pull/3671 https://github.com/preactjs/preact/pull/3671

lioeters 4y ago

Good catch. If this were an internal library, it wouldn't have passed code review.

newbieuser 4y ago

the best vocabulary to describe the monkey patching react ecosystem