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

[BUG]: Error resolving commonjs request for [project]/node_modules/better-sqlite3/lib/database.js #249

Closed
aretrace opened this issue Mar 11, 2023 · 9 comments
Assignees
Labels
bug Something isn't working

Comments

@aretrace
Copy link

aretrace commented Mar 11, 2023

What version of drizzle-orm are you using?

0.21.1

Describe the Bug

I am attempting to use Next.js 13 new app directory (trying out RSCs) with drizzle-orm/better-sqlite3, I get the following error:

... warn - ./node_modules/better-sqlite3/lib/database.js Critical dependency: the request of a dependency is an expression ... Import trace for requested module: ./node_modules/better-sqlite3/lib/database.js ./node_modules/better-sqlite3/lib/index.js ./src/db/client.ts ./src/app/page.tsx error - Failed to generate static paths for /: TypeError: Cannot read properties of undefined (reading 'indexOf') ...

My codebase is a bare-bones skeleton:

src/
├── app/
│ ├── ...
│ ├── page.tsx
│ └── ...
└── db/
├── ...
├── client.ts
└── ...

In client.ts I export a const db from drizzle(...) api and I import it in page.tsx where I attempt to execute a simple query (trying both outside and inside the react component function), the error stems from the instantiation of better-sqlite3's new Database with nextjs displaying the following error in the browser:

Server Error TypeError: Cannot read properties of undefined (reading 'indexOf')
...
export const db = new Database('sqlite.db')
^

I am fairly confident all statements are running only on the server (I used the 'use server' directive and get the same error). Am I missing something here?

@aretrace aretrace added the bug Something isn't working label Mar 11, 2023
@dankochetov
Copy link
Contributor

From your description, I don't see anything related to Drizzle, just to better-sqlite3. Does it work without Drizzle?

@dankochetov dankochetov self-assigned this Mar 12, 2023
@aretrace
Copy link
Author

It does not work without Drizzle, I've opened an issue with better-sqlite3. I would like any ideas as to what's causing the error (reproducible with just using Next.js 13 app dir) but feel free to close the issue if it's out of project scope.

@aretrace
Copy link
Author

Got rid of the error by placing the following in next.config.js:

webpack: (config) => {
    config.externals = {
      'better-sqlite3': 'commonjs better-sqlite3',
    }
    return config
  },

Further discussion can be found at WiseLibs/better-sqlite3#967

@barthicus
Copy link
Contributor

Hi @aretrace . I encountered a similar problem but with mysql: sidorares/node-mysql2#1885 (comment)

I noticed you already solved the issue but in my situation I would prefer to not mess with webpack in next.config.js to prevent future issues and upgrade problems (like moving to turbopack) so instead I was searching for a solution that somehow uses next.js native mechanic.

My stack: next 13 appDir, drizzle orm with mysql, monorepo, turbo, typescript.
And after testing and failing I think I found something that should work in this case.

I put the mysql2 dependency in next.config.js under the experimental.serverComponentsExternalPackages and after server restart it worked. Maybe the same solution would work here? Like instead of mysql2 you could put better-sqlite3 in the serverComponentsExternalPackages array.

So the next.config.js should look like this:

/** @type {import("next").NextConfig} */
const config = {
  reactStrictMode: true,
  experimental: {
    appDir: true,
    typedRoutes: true,
    serverComponentsExternalPackages: ["better-sqlite3"],
    // runtime: "edge",
  },
}
export default config

@aretrace
Copy link
Author

Thank you for the solution @barthicus, serverComponentsExternalPackages is more of a future-proof solution. I am wondering, where would this fix best be documented for others to use?

@barthicus
Copy link
Contributor

@aretrace Probably in some sqlite related readme like this? https://github.com/drizzle-team/drizzle-orm/blob/main/drizzle-orm/src/sqlite-core/README.md

I would just add a section for Next.js 13 users using new app router at the end of the readme and maybe add some alert at the beginning of the readme linking to that section.

BTW I wouldn't be surprised if next team added that dependency (better-sqlite3) to their automatically opted-out list soon. Because sqlite3 lib is already there: https://github.com/vercel/next.js/blob/canary/packages/next/src/lib/server-external-packages.json

@AndriiSherman
Copy link
Member

@barthicus feel free to create a PR for docs improvement. We will check you suggestion and if everything is good with it - will add to docs and mention you in release notes

@barthicus
Copy link
Contributor

@AndriiSherman PR created: #296

@aretrace
Copy link
Author

BTW I wouldn't be surprised if next team added that dependency (better-sqlite3) to their automatically opted-out list soon.

Soon will be arriving shortly: vercel/next.js#47327

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

No branches or pull requests

4 participants