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

Fixes and improvements for some of Qwik examples #233

Merged
merged 9 commits into from
May 4, 2024

Conversation

octet-stream
Copy link
Contributor

@octet-stream octet-stream commented Apr 27, 2024

I've come across this website (this whole thing is a great idea btw) and while looking at the examples for Qwik I found some of them has issues that either cause errors, or they just overcomplicated. This PR fixes and improves upon these examples. Namely:

  1. Declare state: This fix is rather a simplification to be more comparable with the competitors (compared React, Vue, Svelte 5 and Qwik);
  2. Update state: This example results in error, because the state must be updated within useTask$ callback when updated on initial render. In this example it will behave like an "on mount" lifecycle hook from other frameworks;
  3. Computed state: This example is just misleading, because Qwik also has useComputed$ function for when you need computed state, just like the other frameworks (I compared with Vue, React, Svelte and Qwik on the site);
  4. Event click: This example gives an error, because Qwik components are split into symbols, so they are not closures as in React. To fix this error, you either inline event handlers (as in their official examples https://qwik.dev/docs/components/overview/#counter-example) or use the $ function to mark a function as lazy-loadable chunk referenced by QRL;
  5. Conditional templating: This example should use the useComputed$ function because it's recommended way to derive computed values from application's state. Again, components are not closures, they are lazy-lodable chunks.
  6. Form input text: Qwik supports two-way binding in some cases, like form inputs with reactive state. This example can be simplified using bind:value property;
  7. Checkbox: As in previous example, we can use Qwik's two-way binding via bind:checked property;
  8. Radio: This example can also use two-way data binding to update individual radio button state
  9. Select: Simplified using two-way binding via bind:value property;
  10. I also replaced useStore with useSignal in every example I've changed, since reactive objects are not necessary in any of those, and also examples for other frameworks use appropriate APIs for state, unlike for Qwik examples.

You can also play with these examples here: https://stackblitz.com/edit/qwik-starter-dxeodc

@octet-stream octet-stream marked this pull request as ready for review April 27, 2024 20:27
@octet-stream
Copy link
Contributor Author

One more thing. When you type event handlers in extracted function in Qwik there's better way to do it. Event handlers in Qwik take two arguments: an event (typically would be Event interface), and target (any Element interface descendant), so if you need a value from underlying element, you can use second argument instead of the first one. Then there's also EventHandler type exposed from @builder.io/qwik package and you can use this type to add types onto your event handlers. It takes 2 type parameters: event type and target element type. Here's how radio button example can be improved:

import { component$, useSignal, $, type EventHandler } from "@builder.io/qwik";

type PickColorHandler = EventHandler<Event, HTMLInputElement>

const PickPill = component$(() => {
  const pickedColor = useSignal("red");

  const handleChange = $<PickColorHandler>((_, {value}) => {
    pickedColor.value = value;
  });

  return (
    <>
      <div>Picked: {pickedColor.value}</div>
      <input
        id="blue-pill"
        checked={pickedColor.value === "blue"}
        type="radio"
        value="blue"
        onChange$={handleChange}
      />
      <label for="blue-pill">Blue pill</label>

      <input
        id="red-pill"
        checked={pickedColor.value === "red"}
        type="radio"
        value="red"
        onChange$={handleChange}
      />
      <label for="red-pill">Red pill</label>
    </>
  );
});

export default PickPill;

For some reason ESlint was complaining about unused variables in event handler (about the first argument) so I decided not to include this change. But now I don't see any error other than VSCode complaining that I need to enable --jsx in order to use this syntax, so I'm gonna add this example to the PR too.

@octet-stream
Copy link
Contributor Author

Actually I found better way to improve radio button example for Qwik, thanks Vue 3 example for inspiration. The trick here is the props order, but it works same way, so no need for manual updates via event handler :)

@matschik matschik merged commit 0a32b62 into matschik:main May 4, 2024
1 check passed
@matschik
Copy link
Owner

matschik commented May 4, 2024

Qwik snippets are so much better, thanks a lot for your contribution ! You really understand the purpose of Component Party

@octet-stream octet-stream deleted the fix/qwik-examples branch May 5, 2024 02:15
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.

None yet

2 participants