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

[🐞] Qwik bundling PrismaClient on the client when it should only be on the server #3736

Closed
steve8708 opened this issue Apr 10, 2023 · 13 comments · Fixed by #3816
Closed
Labels
P4: urgent If Bug - it makes Qwik unstable and affects the majority of users STATUS-1: needs triage New issue which needs to be triaged TYPE: bug Something isn't working

Comments

@steve8708
Copy link
Contributor

steve8708 commented Apr 10, 2023

Which component is affected?

Qwik Optimizer

Describe the bug

I am trying to use prisma with all of the db calls in server$(), but can't because it is being bundled into the browser and throwing errors

Reproduction

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

Steps to reproduce

Simply load / in the reproduction above. You will see that "DbClient" will be instantiated in the browser, and throw errors, for instance on click.

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

class DbClient {
  constructor() {
    if (typeof window !== 'undefined') {
      throw new Error('DB client on client!');
    }
  }

  get() {
    return [];
  }
}

const dbClient = new DbClient();

const foo = server$(() => {
  return dbClient.get(); // Throws error 'DB client on client'
});

export default component$(() => {
  return (
    <>
      <button
        onClick$={() => {
          foo();
        }}
      >
        Click me
      </button>
    </>
  );
});

System Info

System:
    OS: macOS 13.0
    CPU: (12) arm64 Apple M2 Pro
    Memory: 110.97 MB / 16.00 GB
    Shell: 5.8.1 - /bin/zsh
  Binaries:
    Node: 18.15.0 - ~/.nvm/versions/node/v18.15.0/bin/node
    Yarn: 1.22.19 - ~/.nvm/versions/node/v18.15.0/bin/yarn
    npm: 9.5.0 - ~/.nvm/versions/node/v18.15.0/bin/npm
  Browsers:
    Chrome: 111.0.5563.146
    Edge: 112.0.1722.34
    Safari: 16.1
  npmPackages:
    @builder.io/qwik: 0.101.0 => 0.101.0 
    @builder.io/qwik-city: ~0.101.0 => 0.101.0 
    undici: 5.21.0 => 5.21.0 
    vite: 4.2.1 => 4.2.1

Additional Information

No response

@steve8708 steve8708 added TYPE: bug Something isn't working STATUS-1: needs triage New issue which needs to be triaged labels Apr 10, 2023
@stackblitz
Copy link

stackblitz bot commented Apr 10, 2023

Fix this issue in StackBlitz Codeflow Start a new pull request in StackBlitz Codeflow.

@swwind
Copy link
Contributor

swwind commented Apr 10, 2023

Move class DbClient and const dbClient to another file (such as ~/utils/db) and import them could solve this.

@steve8708
Copy link
Contributor Author

tried it same, issue

@steve8708
Copy link
Contributor Author

heres an updated starter where I tried a few variations. in my project moving to a new file gave the same errors, in this case I'm actually getting a different error (one I've been hitting in my project too) https://stackblitz.com/edit/qwik-starter-3lfrzv?file=src%2Froutes%2Findex.tsx,src%2Fnew-file.ts,src%2Froutes%2Flayout.tsx

Uncaught (in promise) SyntaxError: Unexpected token '<', "<!DOCTYPE "... is not valid JSON
    at JSON.parse (<anonymous>)
    at _deserializeData (resume.js:43:22)
    at serverQrl_stuff_wOIPfiQ04l4 (index.qwik.mjs:775:27)
    at async invokeQRL (qrl-class.js:81:24)
_deserializeData @ resume.js:43
serverQrl_stuff_wOIPfiQ04l4 @ index.qwik.mjs:775
await in serverQrl_stuff_wOIPfiQ04l4 (async)
routes_component__Fragment_button_onClick_BN0XWboDYVI @ index.tsx:14
dispatch @ (index):67
await in dispatch (async)
processDocumentEvent @ (index):94
Show 1 more frame

@marcus-sa
Copy link

We have the same issue.
We'll stick to Remix.

@manucorporat
Copy link
Contributor

I wonder how Remix solves it, how can we know we need to remove this line:

const dbClient = new DbClient();

?

@steve8708
Copy link
Contributor Author

I wonder if we could tag it somehow like // @pure

looking at rollup and other places I see that no one assumes a class instantiation will not have side effects, so we can always close this since it isn't necessarily safe to assume that can be yoinked automatically

it's not hard to do new PrismaClient() within server$() each time you need it

feel free to close out if you like, this isn't really an issue so much as a hypothetical feature suggestion really

@steve8708
Copy link
Contributor Author

actually it turns out prisma really doesn't like it when I create all these clients, I keep getting these logs

warn(prisma-client) There are already 10 instances of Prisma Client actively running.
warn(prisma-client) There are already 10 instances of Prisma Client actively running.

I can help thing on some kind of alternative pattern. maybe something like:

const client = server$(() => new Dbclient())()

not actually sure what would happen if I had the above in a file. I guess I could alternatively do something like

const client = isBrowser ? null : new DbClient()

or something

@y471n
Copy link

y471n commented Apr 14, 2023

Any clues on any hacks to get this working before Qwik team fixes this.

@swwind
Copy link
Contributor

swwind commented Apr 14, 2023

heres an updated starter where I tried a few variations. in my project moving to a new file gave the same errors, in this case I'm actually getting a different error (one I've been hitting in my project too) https://stackblitz.com/edit/qwik-starter-3lfrzv?file=src%2Froutes%2Findex.tsx,src%2Fnew-file.ts,src%2Froutes%2Flayout.tsx

@steve8708 I can get your example work with following changes:

// routes/index.ts
import { dbClient } from '~/new-file';

const foo = server$(() => {
  return dbClient.get();
});

export default component$(() => { ... });
// new-file.ts
class DbClient {
  constructor() {
    if (typeof window !== 'undefined') {
      throw new Error('DB client on client!');
    }
  }

  get() {
    console.log('ran fine');
    return [];
  }
}

export const dbClient = new DbClient();

I forked your stackblitz but it throws a Cross-Site POST error. I believe it's a stackblitz bug, you can clone it and test it locally.

@manucorporat manucorporat added the P4: urgent If Bug - it makes Qwik unstable and affects the majority of users label Apr 17, 2023
manucorporat added a commit that referenced this issue Apr 18, 2023
@marcus-sa
Copy link

marcus-sa commented Apr 18, 2023

I wonder how Remix solves it, how can we know we need to remove this line:

const dbClient = new DbClient();

?

You need to append file names with .server.ts

@steve8708
Copy link
Contributor Author

oh that's an interesting pattern, like if people want additional guarantees/comfort

@Thomva
Copy link

Thomva commented May 21, 2023

I have the same issue. When deploying to Vercel, Netlify or Cloudflare Pages, I get
Error: PrismaClient is unable to be run in the browser.

Happens with the example Prisma schema/code you get after you add the prisma integration. I've tried to use custom schemes, different Prisma providers, routeLoader$, custom api endpoints, etc.

(I have no issues when running the project locally via npm run dev or npm run preview)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
P4: urgent If Bug - it makes Qwik unstable and affects the majority of users STATUS-1: needs triage New issue which needs to be triaged TYPE: bug Something isn't working
Projects
None yet
Development

Successfully merging a pull request may close this issue.

6 participants