[🐞] Qwik is bundling server side dependencies with client code

[DustinJSilk]

Which component is affected?

Qwik Runtime

Describe the bug

Qwik is not effectively treeshaking server side code when bundling for the browser with production or preview builds.

This has been happening since before v1.5.7, however, it was not causing an error as the bundled server code was not loaded in the browser. This can be seen by the fact that there is a 500kb client bundle on version 1.5.7 but no browser error.

Since v.1.8.0, an error occurs because the code is now bundled differently and runs in the browser which crashes the website.

This is the code which results in an error:

import { component$, useVisibleTask$ } from "@builder.io/qwik";
import { server$ } from "@builder.io/qwik-city";
import { SomeEnum } from "~/generated/some/v1/enum_pb";
import { createConnectTransport } from "@connectrpc/connect-node";
import { createPromiseClient } from "@connectrpc/connect";
import { SomeService } from "~/generated/some/v1/service_connect";

// Required to cache the services
class Grpc {
  private static transport = createConnectTransport({
    httpVersion: "2",
    baseUrl: "https://www.test.com",
  });

  static service = createPromiseClient(SomeService, this.transport);
}

export const fooServerFunc = server$(async function () {
 // Calling some server side code
  await Grpc.service.someFetch({});
});

export default component$(() => {
 // Without calling the server$, the error doesn't occur:
  useVisibleTask$(() => {
    fooServerFunc();
  });

 // Without this, the error doesn't occur:
  return <div>{Math.random() == SomeEnum.A ? 1 : 2}</div>;
});

Reproduction

https://github.com/DustinJSilk/qwik-preview-url/blob/main/src/routes/index.tsx

Steps to reproduce

Clone the repo
Run pnpm preview
View error in browser

System Info

N/A

Additional Information

No response

[DustinJSilk]

Instead of using a static class, calling a function which caches the services on the global object solves the issue when using a server$ function. Then to solve any further issues with a formAction$ from modular forms, add this line:

  if (isBrowser) throw new Error('...')

[wmertens]

Ok so in 1.8.0 I upgraded Rust and had do rewrite some logic, including the side effects visitor. Looks like it wasn't done quite correctly, or maybe it's more correct now. Looking into it.

[DustinJSilk]

Ye it might be more correct now! Everything seems to be working quite well now with the above fixes

[wmertens]

So I checked and the tree shaking works but what happens is that the class definition is kept in the main .js file and exported for the server function. The server function actually gets built on the client (but never used)

I think if we can empty out server functions in the client build it will be ok.

To do that, I think the qrl extraction can get a flag not to emit any code. It still needs to walk the code to find other qrls that are used, like when returning a function from a server$ call.

[wmertens]

I was wrong, it does not output the server functions. What happens is that after doing the segmentation the class definition is left in the file.

So I'm going to close this one, because the tree shaking is working correctly, it's just that SWC won't clear an unused class. I opened swc-project/swc#9808, maybe that will get fixed.

[DustinJSilk]

Thanks for the update! That makes sense and it’s simple enough to work around until swc can get it fixed.

[wmertens]

@DustinJSilk see the linked issue, the reason was actually that you store the transport stuff as statics, thus creating side effects. If you don't do that, it removes the code.

[wmertens]

@DustinJSilk so here's how to make it work correctly, you only initialize the statics on the server:

import { component$, useVisibleTask$, isServer } from "@builder.io/qwik";

// ... rest

// Note the isServer guards
class Grpc {
  private static transport = isServer ? createConnectTransport({
    httpVersion: "2",
    baseUrl: "https://www.test.com",
  }) : null;

  static service = isServer ? createPromiseClient(SomeService, this.transport) : null;
}

// ... rest