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鈥檒l occasionally send you account related emails.

Already on GitHub? Sign in to your account

馃捑 Support D1 #277

Closed
mrbbot opened this issue Jun 9, 2022 · 8 comments
Closed

馃捑 Support D1 #277

mrbbot opened this issue Jun 9, 2022 · 8 comments
Assignees
Labels
new api Unsupported new Workers runtime API
Milestone

Comments

@mrbbot
Copy link
Contributor

mrbbot commented Jun 9, 2022

https://blog.cloudflare.com/introducing-d1/

@mrbbot mrbbot added the new api Unsupported new Workers runtime API label Jun 9, 2022
@mrbbot mrbbot added this to the 2.6.0 milestone Jun 9, 2022
@mrbbot mrbbot modified the milestones: 2.6.0, 2.7.0 Jul 9, 2022
@mrbbot mrbbot modified the milestones: 2.7.0, 2.8.0 Aug 19, 2022
@mrbbot mrbbot removed this from the 2.8.0 milestone Sep 3, 2022
@mrbbot mrbbot mentioned this issue Sep 13, 2022
@mrbbot mrbbot added this to the 2.9.0 milestone Sep 13, 2022
@mrbbot mrbbot closed this as completed Sep 16, 2022
@ben-xD
Copy link

ben-xD commented Dec 15, 2022

I'm trying miniflare with a project (wrangler 2.6.2, miniflare 2.11.0) that already uses D1 (D1 alpha, which came after the private beta 馃), but I get:

Not injecting D1 Database for 'DB' as this version of Miniflare only supports D1 beta bindings. Upgrade Wrangler and/or Miniflare and try again.

Is this expected? 馃檹


My wrangler.toml contains:

[[d1_databases]]
binding = "DB"
database_name = "name
database_id = "..."
preview_database_id = "..."

@mrbbot
Copy link
Contributor Author

mrbbot commented Jan 3, 2023

Hey! 馃憢 Happy new year! Just catching up on issues now. 馃檪
Would you be able to share how you're running your Worker? Are you using Miniflare directly, or running via wrangler dev --local?

@ben-xD
Copy link

ben-xD commented Jan 3, 2023

Hey :), happy new year, thanks for your reply.

My attempt at using Miniflare is in 122dacb240c1b359677d5a1032999f27a5ea8706 ben-xD/bounding_box_annotation_flutter@122dacb

I was using miniflare --watch --debug --modules, which now gets a different error:

It works fine with pnpm run dev (which is wrangler dev --env=preview).

Finished custom build steps
[mf:inf] Build succeeded
TypeError: The "path" argument must be of type string. Received an instance of Array
    at new NodeError (node:internal/errors:393:5)
    at validateString (node:internal/validators:163:11)
    at Object.resolve (node:path:1098:7)
    at /Users/zen/Library/pnpm/global/5/.pnpm/@miniflare+core@2.11.0/node_modules/@miniflare/core/src/plugins/build.ts:88:16
    at Array.map (<anonymous>)
    at ChildProcess.<anonymous> (/Users/zen/Library/pnpm/global/5/.pnpm/@miniflare+core@2.11.0/node_modules/@miniflare/core/src/plugins/build.ts:87:45)
    at ChildProcess.emit (node:events:513:28)
    at ChildProcess.emit (node:domain:489:12)
    at Process.ChildProcess._handle.onexit (node:internal/child_process:291:12)

@mrbbot
Copy link
Contributor Author

mrbbot commented Jan 4, 2023

Hey! The problem here is that Miniflare doesn't support using string arrays for [build] watch_dir. Looks like Wrangler 2 added support for this in cloudflare/workers-sdk#1231, but Miniflare never implemented it. 馃檨

Miniflare does have the non-standard [miniflare] build_watch_dirs which it looks like you've tried earlier in the file. Adding...

[miniflare]
build_watch_dirs = ["rust/src", "src"]

to the bottom of your wrangler.toml, removing [build] watch_dir, and running miniflare --watch --debug --modules dist/index.mjs (note the explicit dist/index.mjs as Miniflare also doesn't support the main wrangler.toml option), seems to get Miniflare running.

ben-xD added a commit to ben-xD/bounding_box_annotation_flutter that referenced this issue Jan 7, 2023
@ben-xD
Copy link

ben-xD commented Jan 7, 2023

That works! Thanks for your help and explanation @mrbbot. 馃馃檹 .

I could even delete the following code, which means I don't need to edit wrangler.toml when switching between wrangler and miniflare:

[wasm_modules]
MODULE1 = "src/wasm/backend_bg.wasm"

For anyone interested, the changes I made is shown in ben-xD/bounding_box_annotation_flutter@e49d85d

@trenta3
Copy link

trenta3 commented Jan 19, 2023

Hi! I'm getting the same log message Not injecting D1 Database for 'D1_DB' as this version of Miniflare only supports D1 beta bindings. Upgrade Wrangler and/or Miniflare and try again. even though the miniflare and wrangler versions are the latest one (miniflare 2.11.0, wrangler 2.8.0).

FYI I'm injecting miniflare inside a SvelteKit application, so I'm using some javascript code like:

import { Miniflare, Log, LogLevel } from "miniflare"
import { dev } from "$app/environment"

export const fallBackPlatformToMiniFlareInDev = async (_platform: App.Platform) => {
    if (!dev || _platform)
	return _platform

    const mf = new Miniflare({
	log: new Log(LogLevel.INFO),
	d1Persist: "./data/d1/",
	d1Databases: ["D1_CLOUDFLARE_EXAMPLE_APPLICATION"],
	globalAsyncIO: true,
	globalTimers: true,
	globalRandom: true,
	script: `
		addEventListener("fetch", (event) => {
			event.waitUntil(Promise.resolve(event.request.url));
			event.respondWith(new Response(event.request.headers.get("X-Message")));
		});
		addEventListener("scheduled", (event) => {
			event.waitUntil(Promise.resolve(event.scheduledTime));
		});
`
    })
    const env = await mf.getBindings()
    const platform: App.Platform = { env }
    return platform
}

It is not clear to me how I should proceed in this case, as I don't have a wrangler.toml.

@AdiRishi
Copy link
Contributor

Just starting to use miniflare with D1 and getting the same warning as @trenta3
Looks like it originates from this line.

Why is it a requirement for us to name our databases this way? Is there a way to get around it?
For production I'd rather not have a string prefix for the database name, is there a way to set it only for testing to get around this check?

@kibertoad
Copy link

@mrbbot Are there plans to remove the DB name check? It doesn't seem to make much sense any more.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
new api Unsupported new Workers runtime API
Projects
None yet
Development

No branches or pull requests

6 participants