[🐞] Defining and calling $server() function inside onClick$ causes Error: Dynamic require of "_.js" is not supported

[linkfang]

Which component is affected?

Qwik Runtime

Describe the bug

Hi,

I am defining a server function inside an onClick event handler and call it below the definition.

import { component$ } from "@builder.io/qwik";
import { server$ } from "@builder.io/qwik-city";

export default component$(() => {
  return (
    <button
      onClick$={() => {
        const serverFn = server$(() => console.log('Hi from server'));
        serverFn();
      }}
    >
      Say Hi!
    </button>
  );
});

In dev mode, everything works perfectly.

But in prod mode, I will get a POST error POST http://localhost:4173/?qfunc=n4X1sOI0zW0 500 (Internal Server Error) with error message saying Error: Dynamic require of "_.js" is not supported.

I was expecting no error, because dev mode works fine and what I did aligns with what Miško did in this video

I found a solution to this issue that might help finding the root cause or a fix.
Basically add another $ to wrap the function assigned to onClick$. But I guess this is not needed as onClick$ should be sufficient already, right? Anyways, hope this helps on debugging, thanks!

    <button
      onClick$={$(() => {
        const serverFn = server$(() => console.log('Hi from server'));
        serverFn();
      })}
    >
      Say Hi!
    </button>

Thanks!

Reproduction

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

Steps to reproduce

1. After open the link, the app should be running in dev mode by default
2. The two buttons work great. Click them and the messages will be shown in terminal
3. Stop the app and run it in prod mode by npm run preview
4. The 2nd button still works, but the 1st button doesn't. After click the 1st button, some error messages will be shown in inspect - console
   

System Info

System:
    OS: Windows 11 10.0.22621
    CPU: (8) x64 Intel(R) Core(TM) i7-1065G7 CPU @ 1.30GHz
    Memory: 2.63 GB / 15.60 GB
  Binaries:
    Node: 20.9.0 - C:\Program Files\nodejs\node.EXE
    npm: 10.1.0 - C:\Program Files\nodejs\npm.CMD
  Browsers:
    Edge: Chromium (119.0.2151.72)
    Internet Explorer: 11.0.22621.1

Additional Information

No response

[mhevery]

OK, that is very bizarre. Thank you for a simple repro.

[sarat1669]

Any update on this?

[PatrickJS]

the workaround is just to make sure the functions have $ wrap around them. looking at the build it looks like the server$ is not being output correctly in the server build. @mhevery this seems like an optimizer issue. will this be an issue in v2?

I made my own repro repo with latest qwik
https://github.com/PatrickJS/qwik-error-dynamic-js-server-jwerqd

[PatrickJS]

@wmertens it looks like the server$ output is not working unless the handler is wrap in $

this is the output when the handler is wrap in $

[PatrickJS]

we probably use eslint to require $ if server$ (or any other $ is being used) for now. This works correctly with one server$ but once you have more than one then it breaks

[PatrickJS]

added this to the docs for now 48abd15

[wmertens]

I missed this. This is still a problem in v2 I expect.
I'll take a look this week

[PatrickJS]

yeah I'm fairly certain it's optimizer issue

[genki]

I have faced this issue on the production env at the version 1.5.6-dev20240611011722
It doesn't occured in dev mode. It has been solved by wrapping by the $

[wmertens]

Ok the reason is this: onClick$ will convert the handler to a noopQrl on the server because it can never be called.

However, this happens before the handler is scanned for any other qrls, and so it misses the server$.

The fix is therefore to still process noopQrls to check if there are any server$ calls in there.
However, this needs some care, it's like a double negative, we will now need a list of functions to keep vs a list to convert to noop. This is especially important to get right, to avoid importing DOM-related librairies.

So I'm wondering if we should even support this, and if the eslint rule shouldn't be done instead? @mhevery ?

[PatrickJS]

@wmertens we should support this because calling server$ in onClick$ is used in all examples of server$ and it's the main use-case. Is there any way to process server$ before we convert to noopQrls or just have the optimizer convert everything to $ then it will work correctly? I think eslint is just a workaround until it's automated correctly

[linkfang]

@wmertens we should support this because calling server$ in onClick$ is used in all examples of server$ and it's the main use-case. Is there any way to process server$ before we convert to noopQrls or just have the optimizer convert everything to $ then it will work correctly? I think eslint is just a workaround until it's automated correctly

Hi @PatrickJS , I just double checked the bug, it only happens when I define the server function inside onClick. So, if I define the server function outside of it, then it will work on both dev and prod. Please check the updated repro here: https://stackblitz.com/edit/qwik-starter-uqskwo?file=src%2Froutes%2Findex.tsx

When run npm run preview, only the 1st button will have the error (works in dev, has error in prod), both the 2nd one (defining the server function inside onClick with an extra $) and the 3rd one (defining the server function outside of the onClick and only call it inside) work fine on both dev and prod.

So, I think using eslint or something to force the user to define the server function outside of onClick is fine to me as a quick fix.

[wmertens]

status: this would be solved by #6871 combined with adding if (false) {} around qrls that will be removed. This way they will be processed and other qrls will be discovered.