Skip to content
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

Fix rendering signals as text when using react-transform #439

Merged
merged 4 commits into from
Nov 16, 2023

Conversation

andrewiggins
Copy link
Member

As suggested by @developit, disable useSignals when the auto tracking hook is installed since components can instead rely on that behavior. Also, add useSignals to the SignalValue component to fix it when used with the babel transform

Copy link

changeset-bot bot commented Nov 15, 2023

🦋 Changeset detected

Latest commit: edf4fae

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

This PR includes changesets to release 2 packages
Name Type
@preact/signals-react-transform Patch
@preact/signals-react 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

Copy link

netlify bot commented Nov 15, 2023

Deploy Preview for preact-signals-demo ready!

Name Link
🔨 Latest commit edf4fae
🔍 Latest deploy log https://app.netlify.com/sites/preact-signals-demo/deploys/65545497e484c900082a89ce
😎 Deploy Preview https://deploy-preview-439--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 configuration.

Copy link
Contributor

github-actions bot commented Nov 15, 2023

Size Change: +600 B (+1%)

Total Size: 80.3 kB

Filename Size Change
docs/dist/assets/client.********.js 46.9 kB +235 B (+1%)
docs/dist/react-********.js 239 B +1 B (0%)
packages/react-transform/dist/signals-*********.js 3.49 kB -30 B (-1%)
packages/react-transform/dist/signals-transform.mjs 2.8 kB -33 B (-1%)
packages/react-transform/dist/signals-transform.umd.js 3.61 kB -31 B (-1%)
packages/react/dist/signals.js 1.55 kB +197 B (+15%) ⚠️
packages/react/dist/signals.mjs 1.55 kB +261 B (+20%) 🚨
ℹ️ View Unchanged
Filename Size
docs/dist/assets/index.********.js 833 B
docs/dist/assets/jsxRuntime.module.********.js 281 B
docs/dist/assets/preact.module.********.js 4.02 kB
docs/dist/assets/signals-core.module.********.js 1.46 kB
docs/dist/assets/signals.module.********.js 2.02 kB
docs/dist/assets/style.********.js 21 B
docs/dist/assets/style.********.css 1.21 kB
docs/dist/basic-********.js 244 B
docs/dist/demos-********.js 3.35 kB
docs/dist/nesting-********.js 1.13 kB
packages/core/dist/signals-core.js 1.54 kB
packages/core/dist/signals-core.mjs 1.56 kB
packages/preact/dist/signals.js 1.27 kB
packages/preact/dist/signals.mjs 1.22 kB

compressed-size-action

@@ -146,11 +146,15 @@ const dispatcherMachinePROD = createMachine({
```
*/

export let isAutoSignalTrackingInstalled = false;

let store: EffectStore | null = null;
let lock = false;
let currentDispatcher: ReactDispatcher | null = null;

function installCurrentDispatcherHook() {
Copy link
Contributor

Choose a reason for hiding this comment

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

this function still called on top level of @preact/signals-react so i there is a difference

Copy link
Member Author

Choose a reason for hiding this comment

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

This PR doesn't change the functionality of @preact/signals-react in that importing that package still hooks into internals. Changing that behavior will come later since it is a break change. This PR is starting to setup some infra to eventually make that behavior (hooking into React internals) opt-in. So no difference at the moment for people using @preact/signals-react but that will change eventually.

@andrewiggins andrewiggins merged commit fb6b050 into main Nov 16, 2023
6 checks passed
@andrewiggins andrewiggins deleted the fix-signal-value-2 branch November 16, 2023 00:38
@github-actions github-actions bot mentioned this pull request Nov 16, 2023
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.

3 participants