[🐞] args of the `component$` are not mutable.

genki commented 4 months ago

Which component is affected?

Qwik Runtime

Describe the bug

The props of the component$ are given as the args of the QRL, so they should be mutable, but they are declared as const to be immutable through the useLexicalScope. This makes mismatch. So if I try to change the value of the args, it causes TypeError: Assignment to constant variable. even if there's no error in compile time.

Reproduction

https://stackblitz.com/edit/qwik-starter-c2m93t?file=src%2Froutes%2Findex.tsx

Steps to reproduce

Please see the link above.

System Info

System:
    OS: macOS 14.4.1
    CPU: (8) arm64 Apple M2
    Memory: 93.02 MB / 24.00 GB
    Shell: 3.6.1 - /opt/homebrew/bin/fish
  Binaries:
    Node: 20.12.1 - /opt/homebrew/opt/node@20/bin/node
    Yarn: 1.22.19 - /opt/homebrew/bin/yarn
    npm: 10.5.0 - /opt/homebrew/opt/node@20/bin/npm
    pnpm: 8.7.5 - ~/Library/pnpm/pnpm
    bun: 1.1.3 - ~/.bun/bin/bun
  Browsers:
    Chrome: 125.0.6422.113
    Safari: 17.4.1
  npmPackages:
    @builder.io/qwik: file:../clone/qwik/packages/qwik/dist => 1.5.3-dev20240507001309 
    @builder.io/qwik-city: file:../clone/qwik/packages/qwik-city/lib => 1.5.3-dev20240507001309 
    typescript: ^5.3.3 => 5.3.3 
    undici: ^5.28.3 => 5.28.4 
    vite: ^5.1.5 => 5.2.11

Additional Information

No response

maiieul commented 4 months ago

I'm not sure, this might be intended behavior to enable lazy-loading/lazy-execution.

May I ask what's your use case? You could use a signal instead:

import {
  Signal,
  component$,
  useSignal,
  useVisibleTask$,
} from '@builder.io/qwik';

type FooProps = {
  testSig: Signal<number>;
};

const Foo = component$<FooProps>(({ testSig }) => {
  // eslint-disable-next-line qwik/no-use-visible-task
  useVisibleTask$(() => {
    testSig.value = ++testSig.value;
  });
  return (
    <>
      <div>{testSig}</div>
    </>
  );
});

export default component$(() => {
  const testSig = useSignal(2);
  return (
    <>
      <Foo testSig={testSig} />
      {/* I moved all logic to another file in-case you want to quickly delete and prototype something */}
      <div style={{ padding: '1rem' }}>Hello World</div>
    </>
  );
});

genki commented 4 months ago

@maiieul Yes, I understand this is intended. But I think it is better if warned before the run such as by the tsc or the linter.

shairez commented 4 months ago

Thanks @genki !

Yes I agree that it's a better DX if typescript or eslint could show an error in the code editor.

If anyone has experience with writing eslint rules and want to take a look at one of our existing rules to tackle this, go ahead! (or if @wmertens knows so some handy TS trick to make it happen, feel free to share 😊 )

So marking it as "PR is welcomed" for now, and we might tackle this after V2

Thanks again!

PatrickJS commented 4 months ago

definitely use signals for this but yeah better lint/errors explaining why is a good idea and maybe we can suggest using signals.

what you want to do is either pass a signal or set the initial value of the signal to the prop value depending on what you're trying to do

genki commented 3 months ago

@PatrickJS Yes, using the signal can avoid this :)

QwikDev / qwik