Conversation
rename add fixtures test test binding - ws connection rename test remove version from fixture add test:ci script eslint ignore dynamic import :thinking run prettify
🦋 Changeset detectedLatest commit: f205694 The changes in this PR will be included in the next version bump. This PR includes changesets to release 5 packages
Not sure what this means? Click here to learn what changesets are. Click here if you're a maintainer who wants to add another changeset to this PR |
create-cloudflare
@cloudflare/kv-asset-handler
miniflare
@cloudflare/pages-shared
@cloudflare/unenv-preset
@cloudflare/vite-plugin
@cloudflare/vitest-pool-workers
@cloudflare/workers-editor-shared
wrangler
commit: |
24a10c6 to
89222bc
Compare
petebacondarwin
left a comment
There was a problem hiding this comment.
Only looked at the first few files. Will review properly in an hour or so.
Co-authored-by: Pete Bacon Darwin <pete@bacondarwin.com>
Co-authored-by: Pete Bacon Darwin <pete@bacondarwin.com>
Co-authored-by: Pete Bacon Darwin <pete@bacondarwin.com>
Co-authored-by: Pete Bacon Darwin <pete@bacondarwin.com>
Co-authored-by: Pete Bacon Darwin <pete@bacondarwin.com>
packages/miniflare/src/workers/browser-rendering/binding.worker.ts
Outdated
Show resolved
Hide resolved
| this.#log.logWithLevel(logLevel, message); | ||
| response = new Response(null, { status: 204 }); | ||
| } else if (url.pathname === "/browser/launch") { | ||
| // Version should be kept in sync with the supported version at https://github.com/cloudflare/puppeteer?tab=readme-ov-file#workers-version-of-puppeteer-core |
There was a problem hiding this comment.
Can you add a similar comment to the puppeteer codebase?
There was a problem hiding this comment.
hum not sure where to add it, I'll ask the maintainers
There was a problem hiding this comment.
I'm still tryong to find out where is the best place to add it, can we merge in the meantime pls?
| this.#log.warn.bind(this.#log) | ||
| ); | ||
|
|
||
| // @ts-expect-error Puppeteer is dynamically installed, and so doesn't have types available |
There was a problem hiding this comment.
I believe you can add puppeteer as a dev-dependency and then grab the type of the import, without impacting the distributable size:
type Puppeteer = typeof import('puppeteer')
There was a problem hiding this comment.
the types clash a bit if I do this? @penalosa is this why we didn't do it? :)
There was a problem hiding this comment.
same with this one. @penalosa @petebacondarwin we can move the discussion in a small wiki if you don't mind merging it this way?
There was a problem hiding this comment.
I didn't even try doing that, to be honest—maybe it'll work? What type clashes were you seeing?
Co-authored-by: Pete Bacon Darwin <pete@bacondarwin.com>
Co-authored-by: Pete Bacon Darwin <pete@bacondarwin.com>
Co-authored-by: Pete Bacon Darwin <pete@bacondarwin.com>
petebacondarwin
left a comment
There was a problem hiding this comment.
Yes happy to merge this, after we run the release today. Thank you for a neat PR.
|
Doesnt seem to work when using the vite plugin, is this only for wrangler dev? |
Fixes BRAPI-43
Add local mode support for Browser Rendering. It support puppeteer and messages < 1MB.