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

fix(core): simplify cloudflare wasm loading #670

Merged
merged 3 commits into from
May 9, 2024

Conversation

hi-ogawa
Copy link
Contributor

@hi-ogawa hi-ogawa commented May 9, 2024

Description

It looks like the exact usage of loadWasm(import('shiki/onig.wasm')) shown in the documentation doesn't work currently https://shiki.style/guide/install#cloudflare-workers.

There is actually a different way to load wasm directly WebAssembly.instantiate as demonstrated in packages/shiki/test/cf.ts and it works. However, this usage seems a little more convoluted, so I'm suggesting to improve loadWasm utility to support loadWasm(import('shiki/onig.wasm') usage like in the documentation.

await loadWasm(obj => WebAssembly.instantiate(wasm, obj))

I tested locally with pnpm -C packages/shiki test:cf, but It looks like this is not checked on CI.
Please let me know if it's desired to have some integration test for cloudflare usage.

Linked Issues

#604

Additional context

I was experimenting with shiki on Vite SSR demo https://github.com/hi-ogawa/reproductions/tree/main/shiki-604-cloudflare and currently my loadWasm looks like a following since I need to switch to import("shiki/onig.wasm") only on cloudflare build:

	await loadWasm(async (info) => {
		if (import.meta.env.VITE_BUILD_CF) {
			const mod = await import("shiki/onig.wasm" as string);
			return WebAssembly.instantiate(mod.default, info);
		} else {
			return import("shiki/wasm");
		}
	});

With this PR's patch, I would be able to always write await loadWasm(import("shiki/wasm")) then setup alias or custom resolution to switch to "shiki/onig.wasm hi-ogawa/reproductions#7.

Copy link

netlify bot commented May 9, 2024

Deploy Preview for shiki-matsu ready!

Name Link
🔨 Latest commit 9ae6fc2
🔍 Latest deploy log https://app.netlify.com/sites/shiki-matsu/deploys/663c568e090374000877f2dc
😎 Deploy Preview https://deploy-preview-670--shiki-matsu.netlify.app
📱 Preview on mobile
Toggle QR Code...

QR Code

Use your smartphone camera to open QR code link.

To edit notification comments on pull requests, go to your Netlify site configuration.

Copy link

netlify bot commented May 9, 2024

Deploy Preview for shiki-next ready!

Name Link
🔨 Latest commit 9ae6fc2
🔍 Latest deploy log https://app.netlify.com/sites/shiki-next/deploys/663c568edd6caa00080827ac
😎 Deploy Preview https://deploy-preview-670--shiki-next.netlify.app
📱 Preview on mobile
Toggle QR Code...

QR Code

Use your smartphone camera to open QR code link.

To edit notification comments on pull requests, go to your Netlify site configuration.

@antfu antfu merged commit cd23932 into shikijs:main May 9, 2024
11 checks passed
@hi-ogawa hi-ogawa deleted the fix-cloudflare-wasm branch May 9, 2024 08:13
diegohaz referenced this pull request in ariakit/ariakit May 14, 2024
[![Mend
Renovate](https://app.renovatebot.com/images/banner.svg)](https://renovatebot.com)

This PR contains the following updates:

| Package | Change | Age | Adoption | Passing | Confidence |
|---|---|---|---|---|---|
| [shiki](https://github.com/shikijs/shiki)
([source](https://github.com/shikijs/shiki/tree/HEAD/packages/shiki))
| [`1.3.0` ->
`1.5.1`](https://renovatebot.com/diffs/npm/shiki/1.3.0/1.5.1) |
[![age](https://developer.mend.io/api/mc/badges/age/npm/shiki/1.5.1?slim=true)](https://docs.renovatebot.com/merge-confidence/)
|
[![adoption](https://developer.mend.io/api/mc/badges/adoption/npm/shiki/1.5.1?slim=true)](https://docs.renovatebot.com/merge-confidence/)
|
[![passing](https://developer.mend.io/api/mc/badges/compatibility/npm/shiki/1.3.0/1.5.1?slim=true)](https://docs.renovatebot.com/merge-confidence/)
|
[![confidence](https://developer.mend.io/api/mc/badges/confidence/npm/shiki/1.3.0/1.5.1?slim=true)](https://docs.renovatebot.com/merge-confidence/)
|

---

### Release Notes

<details>
<summary>shikijs/shiki (shiki)</summary>

### [`v1.5.1`](https://github.com/shikijs/shiki/releases/tag/v1.5.1)

[Compare
Source](https://github.com/shikijs/shiki/compare/v1.5.0...v1.5.1)

#####    🐞 Bug Fixes

- **core**: Simplify cloudflare wasm loading  -  by
[@&#8203;hi-ogawa](https://github.com/hi-ogawa) in
[https://github.com/shikijs/shiki/issues/670](https://github.com/shikijs/shiki/issues/670)
[<samp>(cd239)</samp>](https://github.com/shikijs/shiki/commit/cd239324)

#####     [View changes on
GitHub](https://github.com/shikijs/shiki/compare/v1.5.0...v1.5.1)

### [`v1.5.0`](https://github.com/shikijs/shiki/releases/tag/v1.5.0)

[Compare
Source](https://github.com/shikijs/shiki/compare/v1.4.0...v1.5.0)

#####    🚀 Features

- Upgrade deps, new langs  -  by
[@&#8203;antfu](https://github.com/antfu)
[<samp>(d5b04)</samp>](https://github.com/shikijs/shiki/commit/d5b04703)

#####     [View changes on
GitHub](https://github.com/shikijs/shiki/compare/v1.4.0...v1.5.0)

### [`v1.4.0`](https://github.com/shikijs/shiki/releases/tag/v1.4.0)

[Compare
Source](https://github.com/shikijs/shiki/compare/v1.3.0...v1.4.0)

#####    🚀 Features

- Upgrade deps, new langs and themes  -  by
[@&#8203;antfu](https://github.com/antfu)
[<samp>(26f37)</samp>](https://github.com/shikijs/shiki/commit/26f37f08)

#####    🐞 Bug Fixes

- **core**: Check existance of `Buffer.isBuffer`, fix
[#&#8203;664](https://github.com/shikijs/shiki/issues/664)  -  by
[@&#8203;NullVoxPopuli](https://github.com/NullVoxPopuli) in
[https://github.com/shikijs/shiki/issues/666](https://github.com/shikijs/shiki/issues/666)
and
[https://github.com/shikijs/shiki/issues/664](https://github.com/shikijs/shiki/issues/664)
[<samp>(86d52)</samp>](https://github.com/shikijs/shiki/commit/86d5271e)
- **monaco**: Options for tokenize limit  -  by
[@&#8203;hddhyq](https://github.com/hddhyq),
[@&#8203;antfu](https://github.com/antfu) and **Anthony Fu** in
[https://github.com/shikijs/shiki/issues/657](https://github.com/shikijs/shiki/issues/657)
[<samp>(a606d)</samp>](https://github.com/shikijs/shiki/commit/a606d449)
- **transformers**: Allow SQL comment syntax in notation transformer,
fix [#&#8203;654](https://github.com/shikijs/shiki/issues/654)  -  by
[@&#8203;senicko](https://github.com/senicko) in
[https://github.com/shikijs/shiki/issues/655](https://github.com/shikijs/shiki/issues/655)
and
[https://github.com/shikijs/shiki/issues/654](https://github.com/shikijs/shiki/issues/654)
[<samp>(cc135)</samp>](https://github.com/shikijs/shiki/commit/cc13539e)

#####     [View changes on
GitHub](https://github.com/shikijs/shiki/compare/v1.3.0...v1.4.0)

</details>

---

### Configuration

📅 **Schedule**: Branch creation - At any time (no schedule defined),
Automerge - At any time (no schedule defined).

🚦 **Automerge**: Disabled by config. Please merge this manually once you
are satisfied.

♻ **Rebasing**: Whenever PR becomes conflicted, or you tick the
rebase/retry checkbox.

🔕 **Ignore**: Close this PR and you won't be reminded about this update
again.

---

- [ ] <!-- rebase-check -->If you want to rebase/retry this PR, check
this box

---

This PR has been generated by [Mend
Renovate](https://www.mend.io/free-developer-tools/renovate/). View
repository job log
[here](https://developer.mend.io/github/ariakit/ariakit).

<!--renovate-debug:eyJjcmVhdGVkSW5WZXIiOiIzNy4zMjEuMiIsInVwZGF0ZWRJblZlciI6IjM3LjM0MC4xMCIsInRhcmdldEJyYW5jaCI6Im1haW4iLCJsYWJlbHMiOltdfQ==-->

Co-authored-by: renovate[bot] <29139614+renovate[bot]@users.noreply.github.com>
@wouterds
Copy link

Tried all sorts of different ways but unable to get this working on Cloudflare with 1.5.2. Is there a Cloudflare example to plug & play deploy?

await loadWasm(import('shiki/wasm'));

const highlighter = await getHighlighterCore({
  themes: [import('shiki/themes/github-light.mjs')],
  langs: [
    import('shiki/langs/javascript.mjs'),
    import('shiki/langs/xml.mjs'),
    import('shiki/langs/java.mjs'),
    import('shiki/langs/bash.mjs'),
    import('shiki/langs/bibtex.mjs'),
  ],
});

results in

✘ [ERROR] service core:user:worker: Uncaught TypeError: Cannot read properties of undefined (reading 'buffer')

@hi-ogawa
Copy link
Contributor Author

@wouterds Did you follow https://shiki.style/guide/install#cloudflare-workers? Note that import('shiki/wasm') is not intended to work but you need import('shiki/onig.wasm').

As written in the description, there is https://github.com/shikijs/shiki/blob/main/packages/shiki/test/cf.ts to test it out locally.
Also I just tested it out in a fresh project https://github.com/hi-ogawa/reproductions/tree/main/shiki-670-cloudflare and it looks working for me.

@wouterds
Copy link

wouterds commented May 15, 2024

Sorry I should have clarified that I'm using Remix with Cloudflare Pages. Eventually I got it working in a similar fashion wouterds/wouterds.be@fedec70 locally. However, my deploy to Cloudflare "succeeds" (wrangler pages deploy gives no error), but then the function fails with an unknown error (8000068) and the Wrangler logs don't tell a lot more.

✘ [ERROR] A request to the Cloudflare API (/accounts/8406f1e07e7516b76b5c2bf6f2b704d8/pages/projects/wouterds/deployments/05465991-1a75-4e90-8ae0-4e92d5743372/tails) failed.

  workers.api.error.unknown [code: 8000068]
  
  If you think this is a bug, please open an issue at:
  https://github.com/cloudflare/workers-sdk/issues/new/choose

I would assume it's some size limitation on the free plan, but even that seems weird as the total size of the build folder is around 3 MB and the biggest file 300 KB, which is well below Cloudflare stated limits.

Edit: hopefully this gives some more insights once merged cloudflare/workers-sdk#5819

Edit 2: turns out it indeed the bundle was too large

330910460-f80415af-de9d-4bab-93f9-7735bd12542c

Edit 3: managed to work around it by loading shiki from esm.sh wouterds/wouterds.be@aba12a2

Edit 4: wrote about it https://wouterds.be/blog/code-highlighting-with-shiki-keeping-your-cloudflare-worker-bundle-small

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Cannot work with hono + Cloudflareworkers
3 participants