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

[🐞] when passing a "string" key props the symbol output is different which seems to impact reactivity #6819

Open
steffanek opened this issue Aug 23, 2024 · 5 comments
Labels
STATUS-1: needs triage New issue which needs to be triaged TYPE: bug Something isn't working

Comments

@steffanek
Copy link

steffanek commented Aug 23, 2024

Which component is affected?

Qwik Optimizer (rust)

Describe the bug

It seems that Symbols output are different when passing a "string" as a key.
Which might affect reactivity when using signals within the component.

import type { PropsOf, QRL, Signal } from '@builder.io/qwik';
import { $, component$, Slot, useSignal, useTask$ } from '@builder.io/qwik';

export type ToggleProps = PropsOf<'button'> & {
  "halo"?: boolean;
};

export const Toggle = component$<ToggleProps>((props) => {
  const {
    "halo": givenValueSig,
    ...buttonProps
  } = props;

  return (
    <button
      type="button"
      {...buttonProps}
      class={props.class}
    >
      <Slot />
    </button>
  );
});

export default component$(() => <Toggle>hello</Toggle>)

The symbol will convert to:

const { "halo": givenValueSig, ...buttonProps } = props;

However if the component look as follow:

import type { PropsOf, QRL, Signal } from '@builder.io/qwik';
import { $, component$, Slot, useSignal, useTask$ } from '@builder.io/qwik';

export type ToggleProps = PropsOf<'button'> & {
  halo?: boolean;
};

export const Toggle = component$<ToggleProps>((props) => {
  const {
    halo: givenValueSig,
    ...buttonProps
  } = props;

  return (
    <button
      type="button"
      {...buttonProps}
      class={props.class}
    >
      <Slot />
    </button>
  );
});

export default component$(() => <Toggle>hello</Toggle>) 

The symbol output looks as follow:

const buttonProps = _restProps(props, [
        "halo"
    ]);

Reproduction

https://github.com/steffanek/qwik-ui/tree/pr-toggle-togglegroup

Steps to reproduce

  1. Clone the fork
  2. Install deps and run dev
  3. Go to: http://localhost:5173/docs/headless/toggle-group/#reactive-value-controlled and see the 2nd example with the "bind:value". The example works as expected.
  4. Now go to /packages/kit-headless/src/components/toggle/toggle.tsx and uncomment line 39: // 'bind:pressed': givenValueSig,
  5. Go again to: http://localhost:5173/docs/headless/toggle-group/#reactive-value-controlled and see the 2nd example with the "bind:value". Now the behavior is broken.

To quickly see the different symbol output see this playground: https://qwik.dev/playground/#f=Q0o0psbwRAOOlmpQEVpQ7J%2BmoxAY5KOjAKmB8aUheMSo6CClH6BOYEbQQVTiYCa4eCQmQYLdEpKfnp6TCnYPMBih7rJRhzRF1O0U1MCBqpSRmJOvZG%2BlkJQPrPcT88A%2BRI0IiEFAMxDus0Ey3E5DA9ysgccTRBeIBTPeClhWlqXmgSs%2BoH90wFJ6enoQt4BNAQrVAq0AmwR0AHpsQ1RCIwbkPVsliJASVKwa1bhaqDC4pWVbDTZWD8yByMDjGBTOsCiHt9OgUQ3EBLM4NCTsMkAJzkYfytMczdhDMGMDAA

System Info

System:
    OS: macOS 14.3
    CPU: (8) arm64 Apple M3
    Memory: 1.21 GB / 24.00 GB
    Shell: 5.9 - /bin/zsh
  Binaries:
    Node: 22.2.0 - /opt/homebrew/bin/node
    npm: 10.7.0 - /opt/homebrew/bin/npm
    pnpm: 9.7.0 - /opt/homebrew/bin/pnpm
  Browsers:
    Safari: 17.3
  npmPackages:
    @builder.io/qwik: 1.7.3 => 1.7.3
    @builder.io/qwik-city: 1.7.3 => 1.7.3
    typescript: 5.4.5 => 5.4.5
    undici: 5.28.4 => 5.28.4
    vite: 5.2.11 => 5.2.11

Additional Information

No response

@steffanek steffanek added STATUS-1: needs triage New issue which needs to be triaged TYPE: bug Something isn't working labels Aug 23, 2024
@JerryWu1234
Copy link
Contributor

Could offer a minimum duplicated ?

@steffanek
Copy link
Author

Could offer a minimum duplicated ?

@JerryWu1234 What do you mean?

@JerryWu1234
Copy link
Contributor

Could offer a minimum duplicated ?

@JerryWu1234 What do you mean?

You offered a whole project, Could you provide the smallest reproducible repository?

@wmertens
Copy link
Member

@JerryWu1234 it's ok, Steff already provided the playground with the issue

@wmertens
Copy link
Member

So the _restProps is here

export const _restProps = (props: Record<string, any>, omit: string[]) => {
const rest: Record<string, any> = {};
for (const key in props) {
if (!omit.includes(key)) {
rest[key] = props[key];
}
}
return rest;
};
and I don't understand why this would cause the example to work and the how the spread props make it fail.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
STATUS-1: needs triage New issue which needs to be triaged TYPE: bug Something isn't working
Projects
None yet
Development

No branches or pull requests

3 participants