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

"require is not defined" when initializing DB with d1 beta bindings #400

Closed
tgriesser opened this issue Oct 1, 2022 · 7 comments
Closed
Assignees
Milestone

Comments

@tgriesser
Copy link

Was trying out the d1 beta bindings in miniflare, and ran into the following:

[mf:err] ReferenceError: require is not defined
    at _resolve (file:///Users/tgriesser/Github/cf-test/node_modules/.pnpm/npx-import@1.1.3/node_modules/npx-import/lib/utils.js:11:5)
    at npxResolve (file:///Users/tgriesser/Github/cf-test/node_modules/.pnpm/npx-import@1.1.3/node_modules/npx-import/lib/index.js:56:16)
    at createSQLiteDB (/Users/tgriesser/Github/cf-test/node_modules/.pnpm/@miniflare+shared@2.9.0/node_modules/@miniflare/shared/src/sqlite.ts:20:51)
    at FileStorage.getSqliteDatabase (/Users/tgriesser/Github/cf-test/node_modules/.pnpm/@miniflare+storage-file@2.9.0/node_modules/@miniflare/storage-file/src/index.ts:114:21)

Seems it's due to this code in npx-import, which is a package of "type": "module":

export function _resolve(packageWithPath: string) {
  return require.resolve(packageWithPath)
}

I believe this needs to be changed to create require:

export function _resolve(packageWithPath: string) {
  return createRequire(import.meta.url).resolve(packageWithPath)
}

Tried to patch and write a test for it in the npx-import package, but couldn't figure out getting vitest to load the test in a way that it'd fail, where require is undefined. Didn't see a way to force load as a true esm environment, seems require is always defined in the worker context.

The other fix that seemed to work was to change:

return new DatabaseConstructor(dbPath, {
nativeBinding: getSQLiteNativeBindingLocation(npxResolve("better-sqlite3")),
});

to just use require.resolve directly:

return new DatabaseConstructor(dbPath, {
  nativeBinding: getSQLiteNativeBindingLocation(require.resolve("better-sqlite3")),
});

Figured @geelen would know better where is best to fix this, or if there was something else I was missing here where the current code is expected.

@CraigglesO
Copy link
Contributor

My understanding is this PR fixes it:
geelen/npx-import#9

@penalosa penalosa moved this to Untriaged in workers-sdk Oct 4, 2022
@tgriesser
Copy link
Author

Yep, that'll fix it

@geelen
Copy link
Contributor

geelen commented Oct 14, 2022

Released npx-import@v1.1.4. Does miniflare/wrangler need to be published to pick this up or nah? Probably wrangler cause it bundles its dependencies, hey...

@penalosa
Copy link
Contributor

@mrbbot I guess this needs a Miniflare release, and then a Wrangler release?

@beeequeue
Copy link

You can add a resolution to your package.json to fix it in the meantime.

  "resolutions": {
    "npx-import": "1.1.4"
  }

@Skye-31
Copy link
Contributor

Skye-31 commented Nov 19, 2022

This should be shipped on any fresh installation of Miniflare/Wrangler - this issue can be closed 🙂

@beeequeue
Copy link

I just installed/updated them when I wrote that comment and had to add the resolution. 😕

@mrbbot mrbbot moved this from Untriaged to Backlog in workers-sdk Nov 28, 2022
@mrbbot mrbbot self-assigned this Nov 28, 2022
@lrapoport-cf lrapoport-cf moved this from Backlog to Selected for Development in workers-sdk Dec 2, 2022
@mrbbot mrbbot modified the milestones: 2.11.0, 2.12.0 Dec 22, 2022
@mrbbot mrbbot closed this as completed in 9040162 Dec 22, 2022
Repository owner moved this from Selected for Development to Done in workers-sdk Dec 22, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

7 participants