search

BearToCode / carta

A lightweight, fast and extensible Svelte Markdown editor and viewer.
https://beartocode.github.io/carta/
MIT License
458 stars 20 forks source link

Cannot run with Cloudflare Pages #78

Closed ItsMrCube closed 4 months ago

ItsMrCube commented 4 months ago

I'm unable to use this package with Cloudflare Pages since the bundle size becomes too large, this is probably due to shiki taking up a large amount of space.

I'd like to mention that I found some documentation that might be helpful, according to the shiki docs it's possible to use a "fine-grained bundle" to reduce the bundle size: shiki.style/guide/install#fine-grained-bundle

I'm using SvelteKit along with the latest version of carta (4.2.1), I've included some error messages along with some info on the bundle size below.

Output of rollup-plugin-visualizer

image

Vite output


.svelte-kit/output/client/_app/immutable/chunks/wasm.CsTmP73Z.js                      622.30 kB │ gzip: 231.14 kB
.svelte-kit/output/client/_app/immutable/chunks/cpp.CiNIqgXH.js                       628.27 kB │ gzip:  48.41 kB
.svelte-kit/output/client/_app/immutable/chunks/emacs-lisp.BEjL32p1.js                807.64 kB │ gzip: 203.35 kB

(!) Some chunks are larger than 500 kB after minification. Consider:

Cloudflare

Error: Failed to publish your Function. Got error: Your Functions script is over the 1 MiB size limit (workers.api.error.script_too_large)

Thank you in advance.

BearToCode commented 4 months ago

Hi! You're right, there should be an option to prevent all those languages to be bundled into the highlighter. I think there should be an option with different strategies for importing languages:

Something similar (maybe except for the fetch part) could be done for themes.

However, this could reopen an issue with plugin-code, which was recently fixed (#75). Also, this may fix this issue here: #54

warren-bank commented 4 months ago

please correct me if this is a crazy or bad idea (I'm still very new to Svelte).. but wouldn't it be possible to wrap all of the places that initialize Shiki in a conditional: if (browser).. and completely tree shake Shiki from the server-side bundle?

related:

doing a quick grep of the source..

..and that's it, although index.ts would probably also need to remove: export * from '$lib/internal/highlight'

so.. in theory, a few minor tweaks and Shiki should get shook.. from the server bundle. any thoughts?

warren-bank commented 4 months ago

PS: although the intention of my previous comment is to prevent including Shiki in the server-side bundle that gets uploaded to and executed by Cloudflare Workers, because (I believe that..) it isn't even used when performing SSR'ing.. the following is info that I found regarding the correct way to do so:

Shiki has a document that specifically addresses this.

Apparently, the way it imports .wasm doesn't work without a little workaround.

deeper dive.. * [shikijs/core/src](https://github.com/shikijs/shiki/tree/v1.7.0/packages/core/src) * [shikijs/shiki/src](https://github.com/shikijs/shiki/tree/v1.7.0/packages/shiki/src) * [shikijs/shiki/src/index.ts](https://github.com/shikijs/shiki/blob/v1.7.0/packages/shiki/src/index.ts#L1) ```js export * from './bundle-full' ``` * [shikijs/shiki/src/bundle-full.ts](https://github.com/shikijs/shiki/blob/v1.7.0/packages/shiki/src/bundle-full.ts#L17-L34) ```js export const getHighlighter = createdBundledHighlighter( bundledLanguages, bundledThemes, getWasmInlined, ) ``` * [shikijs/shiki/src/bundle-full.ts](https://github.com/shikijs/shiki/blob/v1.7.0/packages/shiki/src/bundle-full.ts#L7) ```js export { getWasmInlined } from './wasm-dynamic' ``` * [shikijs/shiki/src/wasm-dynamic.ts](https://github.com/shikijs/shiki/blob/v1.7.0/packages/shiki/src/wasm-dynamic.ts) ```js export const getWasmInlined: WebAssemblyInstantiator = async (info) => { return import('shiki/wasm').then(wasm => wasm.default(info)) } ``` * [shikijs/shiki/src/wasm.ts](https://github.com/shikijs/shiki/blob/v1.7.0/packages/shiki/src/wasm.ts) ```js export { default } from '@shikijs/core/wasm-inlined' ``` * [shikijs/core/src/wasm-inlined.ts](https://github.com/shikijs/shiki/blob/v1.7.0/packages/core/src/wasm-inlined.ts) ```js import binary from 'vscode-oniguruma/release/onig.wasm' export const wasmBinary = binary as ArrayBuffer export default const getWasmInstance: WebAssemblyInstantiator = async (info) => { return WebAssembly.instantiate(wasmBinary, info).then(wasm => wasm.instance.exports) } ``` * [cloudflare: issue](https://community.cloudflare.com/t/fixed-cloudflare-workers-slow-with-moderate-sized-webassembly-bindings/184668/3)

Although, I should probably point out that this workaround still involves importing a .wasm file that is 455 KB:

update: conflicting info

KyleFontenot commented 4 months ago

I'm having the same issue as well now. I'm just making a bloody mess chopping Carta up with patch-package to essentially disable the highlighting.

I'm not seeing a way to use shiki on Cloudflare at all if it depends on a wasm, because like the link warren-bank shared , that is for setting up a separate Worker instead of the Pages Functions bundling. (along with the Shiki doc linked).

Edit: Looks like even @shiki/core requires the wasm. So shiki simply makes it impossible for Cloudflare Pages unless theres a way of linking the wasm to a separate Worker.

BearToCode commented 4 months ago

Hello, thanks everyone for the help.

I'll be quite busy for a couple of days, but then I could roll out a fix that would allow using an custom bundle (getHighlighterCore instead of getHighlighter). I would have to add some sort of check to make sure plugin-code does not try to load a language that is not bundled.

Replying to @warren-bank > please correct me if this is a crazy or bad idea (I'm still very new to Svelte).. but wouldn't it be possible to wrap all of the places that initialize Shiki in a conditional: `if (browser)`.. and completely tree shake Shiki from the server-side bundle? > > related: > > * [doc](https://kit.svelte.dev/docs/faq#how-do-i-use-x-with-sveltekit-how-do-i-use-a-client-side-only-library-that-depends-on-document-or-window) > > * [issue](https://github.com/sveltejs/kit/issues/2468) > > > doing a quick grep of the source.. > > * `highlighter.ts` > > * imports `getHighlighter` from `shiki` > * exports `loadHighlighter`, which is the one and only place that calls `getHighlighter` > > * `carta.ts` > > * has a public method `highlighter()`, which is the one and only place that calls `loadHighlighter` > > * `Input.svelte` > > * calls `carta.highlighter()` in: `highlight()` and `highlightNestedLanguages()` > * neither of which are export > * called only from: > > * `onMount()` > > * which isn't called server-side and in the process of tree shaking gets replaced by a no-op > * `$: value; ...` > > * which could easily be updated to: `if (browser) onValueChange(value)` > > * `carta.ts` > > * has a public method `render()`, which calls `carta.highlighter()` > > * `Renderer.svelte` > > * calls `carta.render()` from `debouncedRenderer` > * which is not exported > * called only from: > > * `$: value; ...` > > * which could easily be updated to: `if (browser) onValueChange(value)` > > * `Markdown.svelte` > > * calls `carta.render()` > * called only from: > > * `onMount()` > > * which isn't called server-side and in the process of tree shaking gets replaced by a no-op > > > ..and that's it, although `index.ts` would probably also need to remove: `export * from '$lib/internal/highlight'` > > so.. in theory, a few minor tweaks and Shiki should get shook.. from the server bundle. any thoughts?

I'm pretty sure this won't work, as even if the languages are only used in the client, they would still have to be bundled, so that the client can request them when needed.

Edit: this should fix the issue with the large bundle size, but we'll have to see about the wasm import support, as I don't know how we could approach that

warren-bank commented 4 months ago

@ItsMrCube , hi.. can I ask you a quick question to clarify the problem, since you know Cloudflare Pages better than I do? It wasn't until I just did some reading that I now realize that CF_P is full-stack; I was under the impression that CF_P was static and CF_W was dynamic, but apparently that changed about 2.5 years ago.

So, if I..

  1. have a SvelteKit project (for example)..
  2. run: 
     npm install
 CF_PAGES=1  # when the project uses 'adapter-auto', and I want to target CF_P
 npm run build

Then..

Which I can test locally, if I..

  1. run:

    del wrangler.toml  # remove a file in the example repo which appears to be VERY out of date (for CF_W)

npm install --save-dev wrangler@2.20.0
npm install --save-dev @miniflare/tre@3.0.0-next.13

./node_modules/.bin/wrangler pages dev .svelte-kit/cloudflare --experimental-local
    • off-topic:
      • I'm using wrangler v2.20.0 because v3.61.0 (the current version) doesn't seem to do anything on my system,
        and I don't feel like debugging it
  2. open URL: http://127.0.0.1:8788/sverdle

The thing that I want to clarify is..

  1. does this 1 MB limit apply only to the file: 
     .svelte-kit/cloudflare/_worker.js
  2. is every other file in this output directory considered an "asset", which has a 25 MB limit?
ItsMrCube commented 4 months ago

@ItsMrCube , hi.. can I ask you a quick question to clarify the problem, since you know Cloudflare Pages better than I do?

It wasn't until I just did some reading that I now realize that CF_P is full-stack;

I was under the impression that CF_P was static and CF_W was dynamic, but apparently that changed about 2.5 years ago.

So, if I..

  1. have a SvelteKit project (for example)..

  2. run:

    
 npm install

 CF_PAGES=1  # when the project uses 'adapter-auto', and I want to target CF_P

 npm run build

Then..

  • this will produce the output directory:

    
.svelte-kit/cloudflare
  • having the structure.. ```text .svelte-kit/cloudflare | 404.html | about.html | favicon.png | index.html | robots.txt | _headers | _routes.json | _worker.js | _worker.js.map | +---sverdle | how-to-play.html | \---_app | version.json | \---immutable +---assets | 0.rgE380Ej.css | 2.Cs8KR-Bb.css | 4.DOkkq0IA.css | 5.CU6psp88.css | fira-mono-cyrillic-400-normal.36-45Uyg.woff2 | fira-mono-cyrillic-400-normal.Dq7SlH2J.woff | fira-mono-cyrillic-ext-400-normal.0xXfcOOq.woff | fira-mono-cyrillic-ext-400-normal.B04YIrm4.woff2 | fira-mono-greek-400-normal.C3zng6O6.woff2 | fira-mono-greek-400-normal.DUeQbRz0.woff | fira-mono-greek-ext-400-normal.BEhC8Nsh.woff | fira-mono-greek-ext-400-normal.CsqI23CO.woff2 | fira-mono-latin-400-normal.DKjLVgQi.woff2 | fira-mono-latin-400-normal.g4W12wf9.woff | fira-mono-latin-ext-400-normal.D6XfiR-_.woff2 | fira-mono-latin-ext-400-normal.lWlD_NAB.woff | svelte-welcome.0pIiHnVF.webp | svelte-welcome.VNiyy3gC.png | _layout.VxX115KI.css | _page.BBloWWo2.css | _page.CU6psp88.css | _page.DOkkq0IA.css | +---chunks | entry.oJCVYmsm.js | index.B-IpIKmi.js | index.Ice1EKvx.js | index.R8ovVqwX.js | scheduler.Dk-snqIU.js | stores.CG-4iMoZ.js | +---entry | app.gAIOTSnC.js | start.DfoGsIKh.js | \---nodes 0.DYViTZ7U.js 1.D9rQhYdI.js 2.QKAvpFF6.js 3.DDQOcn6V.js 4.B6WkqrZ0.js 5.DtMLlR0M.js ```

Which I can test locally, if I..

  1. run:

    
del wrangler.toml  # remove a file in the example repo which appears to be VERY out of date (for CF_W)

npm install --save-dev wrangler@2.20.0

npm install --save-dev @miniflare/tre@3.0.0-next.13

./node_modules/.bin/wrangler pages dev .svelte-kit/cloudflare --experimental-local

    • off-topic:

      • I'm using wrangler v2.20.0 because v3.61.0 (the current version) doesn't seem to do anything on my system,
        and I don't feel like debugging it

  2. open URL:

    http://127.0.0.1:8788/sverdle

The thing that I want to clarify is..

  1. does this 1 MB limit apply only to the file:

    
 .svelte-kit/cloudflare/_worker.js

  2. is every other file in this output directory considered an "asset",

    which has a 25 MB limit?

Hi, I'm very sorry but I have no idea if the 1mb limit applies only to that file, this is the first time I've encountered an error like this and I'm pretty clueless about it. I've made a quick search and found out that, according to this post, you can have how many files ad you'd like, as long as they're all under 25mb. I can't really give you a definitive answer for your second question, I think it's a "yes" but I don't have a computer in front of me at this time.

As a side note, this might help you verify if your pages projects is within the limits: https://developers.cloudflare.com/workers/platform/limits/#worker-size

I'll make sure to research this topic more thoroughly tomorrow as it's really late for me at the moment.

KyleFontenot commented 4 months ago

Hey there. Yes the 1MB limit only pertains to the _workers.js file specifically. (After compression. And it may have bumped up to 1.5MB)

So if you want to try externalizing the WASM with Vite as an asset, might be worth trying. Not sure if the adapter will bundle it anyways because its a module import.

As it is right now, if we can import the shiki wasm source from a separate Cloudflare Worker, or perhaps import it as an asset, and then remove most of the bundled languages, it'd work. When I first pushed to Cloudflare Pages yesterday, my _workers.js file was 11MB. It took completely removing the wasm, all the languages and nulling some of Carta's highlighting references, to be able to build and deploy successfully.

warren-bank commented 4 months ago

@KyleFontenot , thanks for the clarification. Knowing that the size of this one file is the bottleneck, I'll do some experimentation with building a minimal SvelteKit project that uses carta.. and watch the effects on its size.

warren-bank commented 4 months ago
common test case * *package.json*: ```json "dependencies": { "@cartamd/plugin-code": "^4.0.5", "carta-md": "^4.2.1" }, ``` * *src/routes/+page.svelte*: ```html ```
test 001 * unmodified - using verbatim copy of `carta-md` package from `npm` * results: - *.svelte-kit/cloudflare/_worker.js*: * size: `10.0 MB`
test 002 * modifications: * *node_modules/carta-md/dist/internal/components/Input.svelte*: - replace: ```js $: highlight(value).then(resize); $: highlightNestedLanguages(value); ``` - with: ```js import {browser} from '$app/environment'; const onValueChange = (value) => { highlight(value).then(resize); highlightNestedLanguages(value); }; $: if (browser) onValueChange(value); ``` * *node_modules/carta-md/dist/internal/components/Renderer.svelte*: - replace: ```js $: debouncedRenderer(value); ``` - with: ```js import {browser} from '$app/environment'; const onValueChange = (value) => { debouncedRenderer(value); }; $: if (browser) onValueChange(value); ``` * results: - *.svelte-kit/cloudflare/_worker.js*: * size: `10.0 MB`
test 003 * extend: `test 002` * modifications: * *node_modules/carta-md/dist/index.js*: - delete: ```js export * from './internal/highlight'; ``` * results: - *.svelte-kit/cloudflare/_worker.js*: * size: `10.0 MB`
test 004 * extend: `test 003` * modifications: * *node_modules/carta-md/dist/internal/carta.js*: - replace: ```js async render(markdown) { const processor = await this.asyncProcessor; const highlighter = await this.highlighter(); await loadNestedLanguages(highlighter, markdown); const dirty = String(await processor.process(markdown)); if (!dirty) return ''; this.dispatcher.dispatchEvent(new CustomEvent('carta-render', { detail: { carta: this } })); return (this.sanitizer && this.sanitizer(dirty)) ?? dirty; } ``` - with: ```js import {browser} from '$app/environment'; async render(markdown) { const processor = await this.asyncProcessor; if (browser) { const highlighter = await this.highlighter(); await loadNestedLanguages(highlighter, markdown); } const dirty = String(await processor.process(markdown)); if (!dirty) return ''; this.dispatcher.dispatchEvent(new CustomEvent('carta-render', { detail: { carta: this } })); return (this.sanitizer && this.sanitizer(dirty)) ?? dirty; } ``` * results: - *.svelte-kit/cloudflare/_worker.js*: * size: `10.0 MB`
test 005 * extend: `test 004` * modifications: * *node_modules/carta-md/dist/internal/carta.js*: - replace: ```js import { loadHighlighter, loadDefaultTheme, loadNestedLanguages } from './highlight'; async highlighter() { if (!this.mHighlighter) { const promise = async () => { return loadHighlighter({ theme: this.theme ?? (await loadDefaultTheme()), grammarRules: this.grammarRules, highlightingRules: this.highlightingRules, shiki: this.shikiOptions }); }; this.mHighlighter = promise(); this.mHighlighter = await this.mHighlighter; } return this.mHighlighter; } ``` - with: ```js let loadNestedLanguages; async highlighter() { if (!browser || this.mHighlighter) return this.mHighlighter; const hl = await import('./highlight'); const {loadHighlighter, loadDefaultTheme} = hl; loadNestedLanguages = hl.loadNestedLanguages; const promise = async () => { return loadHighlighter({ theme: this.theme ?? (await loadDefaultTheme()), grammarRules: this.grammarRules, highlightingRules: this.highlightingRules, shiki: this.shikiOptions }); }; this.mHighlighter = promise(); this.mHighlighter = await this.mHighlighter; return this.mHighlighter; } ``` * results: - *.svelte-kit/cloudflare/_worker.js*: * size: `693 KB` - URL: _http://127.0.0.1:8788/_ * works perfectly - with both _SSR_ and _CSR_ enabled
test 006 * revert: `test 003` * re-add: * *node_modules/carta-md/dist/index.js*: - delete: ```js export * from './internal/highlight'; ``` * results: - no change - *.svelte-kit/cloudflare/_worker.js*: * size: `693 KB`

summary:

warren-bank commented 4 months ago

PR#84 adds the changes derived from test 006.

I built this version of carta-md and copied its dist/ into the same minimal SvelteKit project used for testing. When compiled with adapter-cloudflare, the resulting _worker.js is still only 693 KB, and the resulting build runs as expected in wrangler.

update: I created a free Cloudflare account and uploaded a demo: here. It's super minimal, but it does include a server-side component.. just to test that it works. The page starts with a MarkdownEditor component, and when the markdown value is non-empty, a submit button is visible. When clicked, the content of the markdown editor is POST to the backend, which is then made available to the frontend. When the frontend sees that markdown has been made available by the backend, it displays its content in a Markdown (readonly) component.

BearToCode commented 4 months ago

Hello everyone, and thanks for the wonderful work.

The idea proposed by @warren-bank in #84 is good, however there's an issue with that: you are no longer able to render the whole content on node. Originally, I never thought that would be a problem, but after using the library to render a respectable amount of content, I noticed that first loading the library and then executing all the plugins takes a good amount of time (sometimes more than a second). That's the reason I recently created the <PreRendered> component (docs), so that content could be rendered once and stored along with the other component, making loading content originally created with Carta extremely fast.

Now, I think that disabling this possibility for everyone "just" because it's not working with cloudflare is a bit too much. An option could certainly be added, but I'd first try the following, in order:

  1. Try and use customizable bundle using highlighterCore;
  2. See if there some config to import the wasm as a static asset;
  3. Add an option disabling the highlighter server side.

I'll now try with option 1 and see how it goes

warren-bank commented 4 months ago

please correct me if I'm wrong.. but my understanding is that:

if so.. then since plugin-code is {execution: 'async'}, this means that this particular plugin (ie: code highlighting) only runs in the browser; I don't understand what SSR functionality you say is being disabled by not loading the shiki library when running plugins on the server. Please explain.

PS: my most recent commit to this PR actually goes one step farther, and prevents the initialization of all async plugins when running on the server. Not only is this more efficient, since there's no need to construct a bunch of plugins that ultimately don't get used.. but it also prevents them from running code on the server that is intended for use only in the browser.

With respect to comparing the behavior of carta when:

I tested this a while back.. against the unmodified repo, to inspect original behavior.. and it confirmed that SSR-only does not perform any code highlighting.

BearToCode commented 4 months ago

Originally that was how it worked, and it still does the same if you only use the <MarkdownEditor> and <Markdown> components. However, if you wanted to export the rendered HTML for any reason(performance, using it somewhere where Svelte is not available, etc..) you would need to use the render method. renderSSR is just a "downgraded" version of that, since you cannot use async code for SSR, otherwise I would have used render there too. You can get around the SSR limitation by rendering the content on SvelteKit server-side load functions.

I've now nearly finished a change that would allow custom bundles, this should prevent the 1MB file limit. If it still causes issues, I'll add an option that completely removes shiki from the server-side bundle(your changes).

BearToCode commented 4 months ago

I opened a new PR #85 , you can now select which bundle you wish to use:

https://github.com/BearToCode/carta/blob/e17221b4ab24a4ca0511981e8e7fe97803dfff35/packages/carta-md/src/lib/internal/highlight.ts#L166

I also noticed that we can specify custom ways to load the wasm, would this be useful?

https://github.com/BearToCode/carta/blob/e17221b4ab24a4ca0511981e8e7fe97803dfff35/packages/carta-md/src/lib/internal/highlight.ts#L172-L174

If you guys could test this and tell me if it now works it would be great!

warren-bank commented 4 months ago

  1. PR #85 test at e17221b w/ bundle: full

    • +page.svelte:

      <script lang="ts">
  import { Carta, MarkdownEditor } from 'carta-md'
  import { code } from '@cartamd/plugin-code'

  import 'carta-md/default.css'

  const carta = new Carta({
    extensions: [
      code()
    ]
  })

  let value
</script>

<MarkdownEditor mode="split" bind:value {carta} />
    • results:
      • .svelte-kit/cloudflare/_worker.js:
      • size: 10.0 MB

  2. PR #85 test at e17221b w/ bundle: core

    • +page.svelte:

      <script lang="ts">
  import { Carta, MarkdownEditor } from 'carta-md'
  import { code } from '@cartamd/plugin-code'

  import 'carta-md/default.css'

  const carta = new Carta({
    extensions: [
      code({
        onError: (e) => {console.log(e.message)}
      })
    ],
    shikiOptions: {
      bundle: 'core'
    }
  })

  let value
</script>

<MarkdownEditor mode="split" bind:value {carta} />
    • results:
      • .svelte-kit/cloudflare/_worker.js:
      • size: 10.0 MB
BearToCode commented 4 months ago

Oh, thank you. That's weird... I guess your PR works because you are using the browser SvelteKit env variable specifically, and svelte-package does some magic stuff removing it from the server bundle. This is also an issue since as we were planning on keeping the library framework-agnostic. Since I'm using just a "normal" variable, the bundler has no way to know that it will keep being core, so it bundles everything.

However, I might have an idea to make it work...

BearToCode commented 4 months ago

Ok, should be working now. If no bundle is specified, no langugages are present in the build directory, so I hope it will work:

<script lang="ts">
     import { Carta, MarkdownEditor } from 'carta-md'
     import { code } from '@cartamd/plugin-code'

     import 'carta-md/default.css'

     const carta = new Carta({
       extensions: [code()],
       shikiOptions: {
         bundle: () => import('carta-md/bundle/web')
       }
     })

     let value
   </script>

   <MarkdownEditor mode="split" bind:value {carta} />

By default the bundle is empty. I did not find a way to use the full bundle by default without having it forcefully added to the final build.

warren-bank commented 4 months ago

  1. PR #85 test at 4621830 w/ bundle: undefined

    • +page.svelte:

      <script lang="ts">
  import { Carta, MarkdownEditor } from 'carta-md'
  import { code } from '@cartamd/plugin-code'

  import 'carta-md/default.css'

  const carta = new Carta({
    extensions: [
      code({
        onError: (e) => {console.log(e.message)}
      })
    ],
    shikiOptions: {
      bundle: undefined
    }
  })

  let value
</script>

<MarkdownEditor mode="split" bind:value {carta} />
    • results:
      • .svelte-kit/cloudflare/_worker.js:
      • size: 1.59 MB
BearToCode commented 4 months ago

Is it still too much?

warren-bank commented 4 months ago

max allowed is 1.0 MB.. and this 1.59 MB does not include:

warren-bank commented 4 months ago

  1. PR #85 test at 4621830 w/ bundle: web

    • +page.svelte:

      <script lang="ts">
  import { Carta, MarkdownEditor } from 'carta-md'
  import { code } from '@cartamd/plugin-code'

  import 'carta-md/default.css'

  const carta = new Carta({
    extensions: [
      code({
        onError: (e) => {console.log(e.message)}
      })
    ],
    shikiOptions: {
      bundle: () => import('carta-md/bundle/web')
    }
  })

  let value
</script>

<MarkdownEditor mode="split" bind:value {carta} />
    • results:
      • error during build:
        [commonjs--resolver] Missing "./bundle/web" specifier in "carta-md" package
    • note:
      • package.json needs to add an export for these new bundles
    • for example: 
      "exports": {
  ".": {
    "types": "./dist/index.d.ts",
    "svelte": "./dist/index.js",
    "import": "./dist/index.js"
  },
  "./default.css": "./dist/default.css",
  "./bundle/full": "./dist/bundle/full.js",
  "./bundle/web": "./dist/bundle/web.js"
}
    • results (after updating package.json):
      • .svelte-kit/cloudflare/_worker.js:
      • size: 10.0 MB
BearToCode commented 4 months ago

Well, I guess that's as much as I could do leaving shiki in the package.

If you update you PR so that:

  1. It makes the changes optional (so that the highlighter is available by default), and still allows carta.render to work server-side;
  2. Does not use $browser.

I'll happily proceed with that.

meta.import.env might be interesting: it looks to be tree-shakable. (Edit: it might be removed during build, it's better to check that. In that case, maybe some vite/rollup plugin may be useful)

PS: I think the error [commonjs--resolver] Missing "./bundle/web" specifier in "carta-md" package is because you have not rebuilt the package; I added the exports for those entrypoints and it was working for me

warren-bank commented 4 months ago

oh shoot, you're right.. I only copied dist to node_modules in the demo SvelteKit/Cloudflare project; package.json wasn't updated.. my bad.

warren-bank commented 4 months ago

can you give an example that shows SSR'ing w/ async plugins?

ie, a demo page that includes:

BearToCode commented 4 months ago

With SvelteKit you can do it this way. You do not need change when the page is renderd, it just renders on the server and use the HTML in the page.

warren-bank commented 4 months ago

ohhh, ok.. reading docs, it's such a crazy idea that it might just work

reminds me of my early days in perl.. where my answer to nearly every noob question was: RTFM

warren-bank commented 4 months ago

how would you feel about adding an optional parameter to every function that currently performs a browser check?

ex:

  render(markdown: string, allowSSR?: boolean = false)

so.. for this use case, specifically.. calling carta.render() directly from a server load function.. it could pass the optional boolean as truthy.

so.. render can say:

  if (!browser && !allowSSR) ...

which keeps the server bundle lean and clean by default.. but could add some delayed initialization when this pattern is used.

presumably, the code bunder would detect this usage pattern, and include shiki in the server bundle if used by the project, but (in the default case) it gets excluded.

PS: I assume that the browser variable gets statically inlined to either true or false when bundled. I don't see any reason to avoid using this feature of SvelteKit.

PPS: If this usage pattern is something you want to fully support, is it worth considering allowing separate shikiOptions for use on the server vs. in the browser? The above methodology would make this easy to implement. For example:

{
  shiki: browser ? this.shikiOptions?.csr : this.shikiOptions?.ssr
}
warren-bank commented 4 months ago

nope..

I pushed the commit to a different branch, because it doesn't work as expected.

The build works great.. but shiki is always included in the server bundle.. even if render(md, /* allowSSR */ true) is never called from server code, which results in a 10 MB file.

PS: I added some tests to that branch that demonstrate its usage..

update:

BearToCode commented 4 months ago

Ehy, thanks for your effort. Trying to change the build based on some static env variable is quite hard.

However I think I found a solution that will make everyone happy, without weird settings. I'll try that as soon as I got some time.

I'll keep you updated!

warren-bank commented 4 months ago

I'd be curious to know what idea you have in mind.

As you indicated, I've made a few new commits.

Based on my tests, the only way to tree shake shiki is if there's absolutely no path of execution by which the server code could possibly use highlighter. It doesn't matter if there is a path of execution and the project doesn't use it. For example, my first attempt was to add a 2nd parameter to carta.render(markdown, allowSSR). The idea was that if a project never tries to render async plugins on the server, then the path of execution that is followed when allowSSR is passed a truthy value isn't used.. and it would be eliminated by the compiler. However, that didn't happen.

The only solution that I could think of was to define global constants at compile time. Code paths of execution that cannot be reached because of these contant values are successfully removed from the bundle. As such, I defined two. Their values are defined in the file: vite.config.js

export default defineConfig({
  plugins: [sveltekit()],

  define: {
    __ENABLE_CARTA_SSR_ASYNC_PLUGINS__ : 'true',
    __ENABLE_CARTA_SSR_HIGHLIGHTER__   : 'false'
  }
});

I'd agree with you.. that requiring additional configuration is annoying. But on the other hand, it's only 4 lines.. and gives the user very fine grain control. Also, strictly speaking, unless a user actually attempts to call carta.render() from SSR.. nothing bad would happen if these flags were left undefined.. though I wouldn't recommend it.

To be clear:

To be fair:

I'd be curious to see whether this size could be reduced farther, possibly with the use of additional globally constant boolean flags.

content of 693 KB _worker.js file..
for a project: ```json "dependencies": { "@cartamd/plugin-code": "^4.0.5", "carta-md": "^4.2.1" } ``` grep'ing *_worker.js* content for: `// node_modules/`
and removing the few modules that are also included in an empty template project..
to produce a pseudo table-of-contents w/ leading line numbers: ```txt 1412, "// node_modules/bail/index.js" 1423, "// node_modules/extend/index.js" 1514, "// node_modules/devlop/lib/default.js" 1522, "// node_modules/is-plain-obj/index.js" 1535, "// node_modules/trough/lib/index.js" 1623, "// node_modules/trough/index.js" 1630, "// node_modules/unist-util-stringify-position/lib/index.js" 1660, "// node_modules/unist-util-stringify-position/index.js" 1667, "// node_modules/vfile-message/lib/index.js" 1809, "// node_modules/vfile-message/index.js" 1816, "// node_modules/vfile/lib/minpath.browser.js" 2034, "// node_modules/vfile/lib/minproc.browser.js" 2045, "// node_modules/vfile/lib/minurl.shared.js" 2057, "// node_modules/vfile/lib/minurl.browser.js" 2106, "// node_modules/vfile/lib/index.js" 2569, "// node_modules/vfile/index.js" 2576, "// node_modules/unified/lib/callable-instance.js" 2608, "// node_modules/unified/lib/index.js" 3256, "// node_modules/unified/index.js" 3263, "// node_modules/mdast-util-to-string/lib/index.js" 3305, "// node_modules/mdast-util-to-string/index.js" 3312, "// node_modules/character-entities/index.js" 5446, "// node_modules/decode-named-character-reference/index.js" 5458, "// node_modules/micromark-util-chunked/index.js" 5497, "// node_modules/micromark-util-combine-extensions/index.js" 5544, "// node_modules/micromark-util-decode-numeric-character-reference/index.js" 5566, "// node_modules/micromark-util-normalize-identifier/index.js" 5575, "// node_modules/micromark-util-character/index.js" 5612, "// node_modules/micromark-util-sanitize-uri/index.js" 5656, "// node_modules/micromark-factory-space/index.js" 5683, "// node_modules/micromark/lib/initialize/content.js" 5744, "// node_modules/micromark/lib/initialize/document.js" 5963, "// node_modules/micromark-util-classify-character/index.js" 5978, "// node_modules/micromark-util-resolve-all/index.js" 5996, "// node_modules/micromark-core-commonmark/lib/attention.js" 6115, "// node_modules/micromark-core-commonmark/lib/autolink.js" 6222, "// node_modules/micromark-core-commonmark/lib/blank-line.js" 6244, "// node_modules/micromark-core-commonmark/lib/block-quote.js" 6309, "// node_modules/micromark-core-commonmark/lib/character-escape.js" 6341, "// node_modules/micromark-core-commonmark/lib/character-reference.js" 6413, "// node_modules/micromark-core-commonmark/lib/code-fenced.js" 6603, "// node_modules/micromark-core-commonmark/lib/code-indented.js" 6674, "// node_modules/micromark-core-commonmark/lib/code-text.js" 6795, "// node_modules/micromark-util-subtokenize/lib/splice-buffer.js" 6996, "// node_modules/micromark-util-subtokenize/index.js" 7146, "// node_modules/micromark-core-commonmark/lib/content.js" 7225, "// node_modules/micromark-factory-destination/index.js" 7322, "// node_modules/micromark-factory-label/index.js" 7389, "// node_modules/micromark-factory-title/index.js" 7457, "// node_modules/micromark-factory-whitespace/index.js" 7486, "// node_modules/micromark-core-commonmark/lib/definition.js" 7584, "// node_modules/micromark-core-commonmark/lib/hard-break-escape.js" 7611, "// node_modules/micromark-core-commonmark/lib/heading-atx.js" 7710, "// node_modules/micromark-util-html-tag-name/index.js" 7782, "// node_modules/micromark-core-commonmark/lib/html-flow.js" 8169, "// node_modules/micromark-core-commonmark/lib/html-text.js" 8482, "// node_modules/micromark-core-commonmark/lib/label-end.js" 8704, "// node_modules/micromark-core-commonmark/lib/label-start-image.js" 8741, "// node_modules/micromark-core-commonmark/lib/label-start-link.js" 8769, "// node_modules/micromark-core-commonmark/lib/line-ending.js" 8790, "// node_modules/micromark-core-commonmark/lib/thematic-break.js" 8836, "// node_modules/micromark-core-commonmark/lib/list.js" 8975, "// node_modules/micromark-core-commonmark/lib/setext-underline.js" 9068, "// node_modules/micromark-core-commonmark/index.js" 9096, "// node_modules/micromark/lib/initialize/flow.js" 9153, "// node_modules/micromark/lib/initialize/text.js" 9298, "// node_modules/micromark/lib/create-tokenizer.js" 9624, "// node_modules/micromark/lib/constructs.js" 9706, "// node_modules/micromark/lib/parse.js" 9743, "// node_modules/micromark/lib/postprocess.js" 9755, "// node_modules/micromark/lib/preprocess.js" 9842, "// node_modules/micromark/index.js" 9851, "// node_modules/micromark-util-decode-string/index.js" 9876, "// node_modules/mdast-util-from-markdown/lib/index.js" 10601, "// node_modules/mdast-util-from-markdown/index.js" 10608, "// node_modules/remark-parse/lib/index.js" 10630, "// node_modules/remark-parse/index.js" 10637, "// node_modules/ccount/index.js" 10656, "// node_modules/mdast-util-find-and-replace/node_modules/escape-string-regexp/index.js" 10668, "// node_modules/unist-util-is/lib/index.js" 10765, "// node_modules/unist-util-is/index.js" 10772, "// node_modules/unist-util-visit-parents/lib/color.js" 10781, "// node_modules/unist-util-visit-parents/lib/index.js" 10865, "// node_modules/unist-util-visit-parents/index.js" 10872, "// node_modules/mdast-util-find-and-replace/lib/index.js" 10984, "// node_modules/mdast-util-find-and-replace/index.js" 10991, "// node_modules/mdast-util-gfm-autolink-literal/lib/index.js" 11150, "// node_modules/mdast-util-gfm-autolink-literal/index.js" 11157, "// node_modules/mdast-util-gfm-footnote/lib/index.js" 11277, "// node_modules/mdast-util-gfm-footnote/index.js" 11284, "// node_modules/mdast-util-gfm-strikethrough/lib/index.js" 11341, "// node_modules/mdast-util-gfm-strikethrough/index.js" 11348, "// node_modules/markdown-table/index.js" 11492, "// node_modules/zwitch/index.js" 11518, "// node_modules/mdast-util-to-markdown/lib/handle/blockquote.js" 11539, "// node_modules/mdast-util-to-markdown/lib/util/pattern-in-scope.js" 11563, "// node_modules/mdast-util-to-markdown/lib/handle/break.js" 11579, "// node_modules/longest-streak/index.js" 11607, "// node_modules/mdast-util-to-markdown/lib/util/format-code-as-indented.js" 11621, "// node_modules/mdast-util-to-markdown/lib/util/check-fence.js" 11636, "// node_modules/mdast-util-to-markdown/lib/handle/code.js" 11695, "// node_modules/mdast-util-to-markdown/lib/util/check-quote.js" 11710, "// node_modules/mdast-util-to-markdown/lib/handle/definition.js" 11771, "// node_modules/mdast-util-to-markdown/lib/util/check-emphasis.js" 11786, "// node_modules/mdast-util-to-markdown/lib/handle/emphasis.js" 11813, "// node_modules/unist-util-visit/lib/index.js" 11841, "// node_modules/unist-util-visit/index.js" 11848, "// node_modules/mdast-util-to-markdown/lib/util/format-heading-as-setext.js" 11868, "// node_modules/mdast-util-to-markdown/lib/handle/heading.js" 11915, "// node_modules/mdast-util-to-markdown/lib/handle/html.js" 11928, "// node_modules/mdast-util-to-markdown/lib/handle/image.js" 11990, "// node_modules/mdast-util-to-markdown/lib/handle/image-reference.js" 12033, "// node_modules/mdast-util-to-markdown/lib/handle/inline-code.js" 12069, "// node_modules/mdast-util-to-markdown/lib/util/format-link-as-autolink.js" 12089, "// node_modules/mdast-util-to-markdown/lib/handle/link.js" 12175, "// node_modules/mdast-util-to-markdown/lib/handle/link-reference.js" 12218, "// node_modules/mdast-util-to-markdown/lib/util/check-bullet.js" 12233, "// node_modules/mdast-util-to-markdown/lib/util/check-bullet-other.js" 12258, "// node_modules/mdast-util-to-markdown/lib/util/check-bullet-ordered.js" 12273, "// node_modules/mdast-util-to-markdown/lib/util/check-rule.js" 12288, "// node_modules/mdast-util-to-markdown/lib/handle/list.js" 12336, "// node_modules/mdast-util-to-markdown/lib/util/check-list-item-indent.js" 12351, "// node_modules/mdast-util-to-markdown/lib/handle/list-item.js" 12386, "// node_modules/mdast-util-to-markdown/lib/handle/paragraph.js" 12400, "// node_modules/mdast-util-phrasing/lib/index.js" 12432, "// node_modules/mdast-util-phrasing/index.js" 12439, "// node_modules/mdast-util-to-markdown/lib/handle/root.js" 12453, "// node_modules/mdast-util-to-markdown/lib/util/check-strong.js" 12468, "// node_modules/mdast-util-to-markdown/lib/handle/strong.js" 12495, "// node_modules/mdast-util-to-markdown/lib/handle/text.js" 12504, "// node_modules/mdast-util-to-markdown/lib/util/check-rule-repetition.js" 12519, "// node_modules/mdast-util-to-markdown/lib/handle/thematic-break.js" 12531, "// node_modules/mdast-util-to-markdown/lib/handle/index.js" 12579, "// node_modules/mdast-util-to-markdown/index.js" 12586, "// node_modules/mdast-util-gfm-table/lib/index.js" 12746, "// node_modules/mdast-util-gfm-table/index.js" 12753, "// node_modules/mdast-util-gfm-task-list-item/lib/index.js" 12832, "// node_modules/mdast-util-gfm-task-list-item/index.js" 12839, "// node_modules/mdast-util-gfm/lib/index.js" 12870, "// node_modules/mdast-util-gfm/index.js" 12877, "// node_modules/micromark-extension-gfm-autolink-literal/lib/syntax.js" 13232, "// node_modules/micromark-extension-gfm-autolink-literal/index.js" 13239, "// node_modules/micromark-extension-gfm-footnote/lib/syntax.js" 13532, "// node_modules/micromark-extension-gfm-footnote/index.js" 13539, "// node_modules/micromark-extension-gfm-strikethrough/lib/syntax.js" 13656, "// node_modules/micromark-extension-gfm-strikethrough/index.js" 13663, "// node_modules/micromark-extension-gfm-table/lib/edit-map.js" 13748, "// node_modules/micromark-extension-gfm-table/lib/infer.js" 13781, "// node_modules/micromark-extension-gfm-table/lib/syntax.js" 14202, "// node_modules/micromark-extension-gfm-table/index.js" 14209, "// node_modules/micromark-extension-gfm-task-list-item/lib/syntax.js" 14293, "// node_modules/micromark-extension-gfm-task-list-item/index.js" 14300, "// node_modules/micromark-extension-gfm/index.js" 14321, "// node_modules/remark-gfm/lib/index.js" 14345, "// node_modules/remark-gfm/index.js" 14352, "// node_modules/mdast-util-to-hast/lib/handlers/blockquote.js" 14369, "// node_modules/mdast-util-to-hast/lib/handlers/break.js" 14381, "// node_modules/mdast-util-to-hast/lib/handlers/code.js" 14409, "// node_modules/mdast-util-to-hast/lib/handlers/delete.js" 14426, "// node_modules/mdast-util-to-hast/lib/handlers/emphasis.js" 14443, "// node_modules/mdast-util-to-hast/lib/handlers/footnote-reference.js" 14487, "// node_modules/mdast-util-to-hast/lib/handlers/heading.js" 14504, "// node_modules/mdast-util-to-hast/lib/handlers/html.js" 14519, "// node_modules/mdast-util-to-hast/lib/revert.js" 14552, "// node_modules/mdast-util-to-hast/lib/handlers/image-reference.js" 14574, "// node_modules/mdast-util-to-hast/lib/handlers/image.js" 14593, "// node_modules/mdast-util-to-hast/lib/handlers/inline-code.js" 14612, "// node_modules/mdast-util-to-hast/lib/handlers/link-reference.js" 14639, "// node_modules/mdast-util-to-hast/lib/handlers/link.js" 14660, "// node_modules/mdast-util-to-hast/lib/handlers/list-item.js" 14728, "// node_modules/mdast-util-to-hast/lib/handlers/list.js" 14758, "// node_modules/mdast-util-to-hast/lib/handlers/paragraph.js" 14775, "// node_modules/mdast-util-to-hast/lib/handlers/root.js" 14787, "// node_modules/mdast-util-to-hast/lib/handlers/strong.js" 14804, "// node_modules/unist-util-position/lib/index.js" 14833, "// node_modules/unist-util-position/index.js" 14840, "// node_modules/mdast-util-to-hast/lib/handlers/table.js" 14883, "// node_modules/mdast-util-to-hast/lib/handlers/table-row.js" 14922, "// node_modules/mdast-util-to-hast/lib/handlers/table-cell.js" 14940, "// node_modules/trim-lines/index.js" 14985, "// node_modules/mdast-util-to-hast/lib/handlers/text.js" 14997, "// node_modules/mdast-util-to-hast/lib/handlers/thematic-break.js" 15014, "// node_modules/mdast-util-to-hast/lib/handlers/index.js" 15077, "// node_modules/@ungap/structured-clone/esm/types.js" 15094, "// node_modules/@ungap/structured-clone/esm/deserialize.js" 15160, "// node_modules/@ungap/structured-clone/esm/serialize.js" 15290, "// node_modules/@ungap/structured-clone/esm/index.js" 15303, "// node_modules/mdast-util-to-hast/lib/footer.js" 15419, "// node_modules/mdast-util-to-hast/lib/state.js" 15572, "// node_modules/mdast-util-to-hast/lib/index.js" 15592, "// node_modules/mdast-util-to-hast/index.js" 15599, "// node_modules/remark-rehype/lib/index.js" 15623, "// node_modules/remark-rehype/index.js" 15630, "// node_modules/html-void-elements/index.js" 15659, "// node_modules/property-information/lib/util/schema.js" 15684, "// node_modules/property-information/lib/util/merge.js" 15701, "// node_modules/property-information/lib/normalize.js" 15710, "// node_modules/property-information/lib/util/info.js" 15738, "// node_modules/property-information/lib/util/types.js" 15766, "// node_modules/property-information/lib/util/defined-info.js" 15802, "// node_modules/property-information/lib/util/create.js" 15836, "// node_modules/property-information/lib/xlink.js" 15859, "// node_modules/property-information/lib/xml.js" 15874, "// node_modules/property-information/lib/util/case-sensitive-transform.js" 15883, "// node_modules/property-information/lib/util/case-insensitive-transform.js" 15893, "// node_modules/property-information/lib/xmlns.js" 15908, "// node_modules/property-information/lib/aria.js" 15973, "// node_modules/property-information/lib/html.js" 16345, "// node_modules/property-information/lib/svg.js" 16915, "// node_modules/property-information/lib/find.js" 16959, "// node_modules/property-information/index.js" 16976, "// node_modules/stringify-entities/lib/core.js" 17029, "// node_modules/stringify-entities/lib/util/to-hexadecimal.js" 17041, "// node_modules/stringify-entities/lib/util/to-decimal.js" 17053, "// node_modules/character-entities-legacy/index.js" 17168, "// node_modules/character-entities-html4/index.js" 17429, "// node_modules/stringify-entities/lib/constant/dangerous.js" 17446, "// node_modules/stringify-entities/lib/util/to-named.js" 17476, "// node_modules/stringify-entities/lib/util/format-smart.js" 17504, "// node_modules/stringify-entities/lib/index.js" 17515, "// node_modules/stringify-entities/index.js" 17522, "// node_modules/hast-util-to-html/lib/handle/comment.js" 17549, "// node_modules/hast-util-to-html/lib/handle/doctype.js" 17559, "// node_modules/comma-separated-tokens/index.js" 17572, "// node_modules/space-separated-tokens/index.js" 17581, "// node_modules/hast-util-whitespace/lib/index.js" 17595, "// node_modules/hast-util-whitespace/index.js" 17602, "// node_modules/hast-util-to-html/lib/omission/util/siblings.js" 17628, "// node_modules/hast-util-to-html/lib/omission/omission.js" 17642, "// node_modules/hast-util-to-html/lib/omission/closing.js" 17737, "// node_modules/hast-util-to-html/lib/omission/opening.js" 17793, "// node_modules/hast-util-to-html/lib/handle/element.js" 17936, "// node_modules/hast-util-to-html/lib/handle/text.js" 17953, "// node_modules/hast-util-to-html/lib/handle/raw.js" 17963, "// node_modules/hast-util-to-html/lib/handle/root.js" 17973, "// node_modules/hast-util-to-html/lib/handle/index.js" 18002, "// node_modules/hast-util-to-html/lib/index.js" 18066, "// node_modules/hast-util-to-html/index.js" 18073, "// node_modules/rehype-stringify/lib/index.js" 18088, "// node_modules/rehype-stringify/index.js" 18095, "// node_modules/hast-util-to-string/lib/index.js" 18121, "// node_modules/hast-util-to-string/index.js" 18128, "// node_modules/@shikijs/rehype/dist/core.mjs" 18206, "// node_modules/@cartamd/plugin-code/dist/index.js" ```
BearToCode commented 4 months ago

Should be working now, this is what I've done:

  1. highlighter is no longer initialized by the Carta class, but is set by the MarkdownEditor component on the onMount callback. This way, it should not be present in the server bundle;
  2. I've split up the Carta class into three classes(bundles):
    • CartaBase: base class, with support for rendering content;
    • CartaBrowser: extends CartaBase, adds support for DOM utilities (e.g. shortcuts, listeners, icons...) and the highlighter;
    • CartaServer: extends CartaBase, and is meant to be used only server side.

By default, the CartaBrowser is the Carta one exported by the package. The others can be found in /bundle/[name]. The browser variant is the one required for the components.

The split into different is not really related to this issue, but I've done that to keep the project a bit more organized.

Without changing anything, you should see the _worker.js file go down in size, as the highlighter is not required in the server. It would be great if you can test that, but I'm confident it will work, as it is basically the same strategy used by @warren-bank

ItsMrCube commented 4 months ago

Hi, I've tried the changes on my project and the worker size went down to around 1.2MB, while it's not quite enough to work with Cloudflare it's an huge improvement from when it weighed more than 10MB. I still haven't looked into what's taking up this space so I'll make sure to check since the issue might be on my side.

PS: I wanted to thank you all for working on this issue with this level of dedication. :heart:

ItsMrCube commented 4 months ago

I just rechecked the sizes for the _worker.js file again, here are the results for my project: Without carta: 588,8KiB With, before the changes: 11MiB With, after the changes: 1.7MiB

(Please ignore the part where, in my previous comment, I said that the worker weighed 1.2MB, that wasn't accurate, sorry for that.)

BearToCode commented 4 months ago

Thanks, is there any way to see what is taking all that space in the bundle? So we can see what's left in there. Maybe using the same tool that you used before?

Edit: you are using PR #85 right?

ItsMrCube commented 4 months ago

Hi, I can confirm that I was doing the testing with the custom-bundle branch that's part of #85, I tried my best to build the package myself and used it in my project (this was my first time using pnpm)

Here's a map of what's taking up space, it refers to a worker file that's 1.7MiB. image Looks like shiki is still taking up most of the space, at least with my project.

BearToCode commented 4 months ago

It seems so, however I think that is the whole bundle, and not the one that goes to the server. In fact, if you hover over the root element of the graph it should say 10MB, which is the original whole bundle(I tried that now myself and it shows that). I wonder if there's a way to only show what make up the worker file... Maybe @warren-bank can help us? As he seemed able to reduce the bundle further

warren-bank commented 4 months ago

@BearToCode , hello.. I haven't had a chance to look at your new version yet. I'll do that this afternoon. Off-hand, I'm not sure that I understand how it would be used to handle the normal use case.. where the same code is used for both SSR and CSR. Anyway.. I'll just need to do a little reading before I can discuss pros and cons.

@ItsMrCube , hello.. for comparison sake, would you be willing to quickly do a test build using the version in PR #84 ? If so, please remember to configure vite.config.js. PS: to simplify, you can use the packages that I've already built: carta-md, plugin-code

warren-bank commented 4 months ago

@BearToCode , early days.. initial thoughts:

quick aside.. kind of a dumb question.. but have you written any tests to see this library in action?

warren-bank commented 4 months ago

I suppose the following could (maybe) work:

  1. add to Markdown.svelte: 
     onMount(async () => {
   const module = await import('$lib/core/highlight');
   highlighter = await module.loadHighlighter({
     grammarRules: carta.grammarRules,
     highlightingRules: carta.highlightingRules,
     theme: carta.theme ?? (await module.loadDefaultTheme()),
     shiki: carta.shikiOptions
   });
   carta.$setHighlighter(highlighter);
 });

  2. add new class: lib/bundle/server-highlighter.js (or whatever)

     export interface ServerHighlighterOptions extends BaseOptions {
   /**
    * Highlighter options.
    */
   shikiOptions?: ShikiOptions;
   /**
    * ShikiJS theme
    * @default 'carta-light' for light mode and 'carta-dark' for dark mode.
    */
   theme?: Theme | DualTheme;
 }

 export class CartaServerHighlighter extends CartaBase {
   public override readonly bundle = () => 'server-highlighter';

   public readonly theme?: Theme | DualTheme;
   public readonly shikiOptions?: ShikiOptions;
   public readonly grammarRules: GrammarRule[];
   public readonly highlightingRules: HighlightingRule[];

   public constructor(options: ServerHighlighterOptions) {
     super(options);

     this.theme = options?.theme;
     this.shikiOptions = options?.shikiOptions;
     this.grammarRules = [];
     this.highlightingRules = [];
   }

   public async init() {
     const module = await import('$lib/core/highlight');
     highlighter = await module.loadHighlighter({
       grammarRules: this.grammarRules,
       highlightingRules: this.highlightingRules,
       theme: this.theme ?? (await module.loadDefaultTheme()),
       shiki: this.shikiOptions
     });
     this.$setHighlighter(highlighter);

     this.initExtensions();
   }
 }

notes:

The design choice is:

This is a question of personal opinion and preference. Personally, I prefer the current design and find that this change is a move in the wrong direction. Open to discussion..

BearToCode commented 4 months ago

You're right, this is becoming more complex than it need to be. I closed my PR and worked a bit on yours: I cleaned it up a bit (I also think your intellisense does not work correctly), I removed the __ENABLE_CARTA_SSR_ASYNC_PLUGINS_, as it didn't seem really necessary.

I tested it locally and it seems to work perfectly! The highlight module is not present in the server bundle.

All you have to do now is edit the vite.config.js:

import { sveltekit } from '@sveltejs/kit/vite';

/** @type {import('vite').UserConfig} */
const config = {
    plugins: [sveltekit()],
    define: {
            __ENABLE_CARTA_SSR_HIGHLIGHTER__   : false
    }
};

export default config;

Good job everyone. Have a nice day!

github-actions[bot] commented 4 months ago

:tada: This issue has been resolved in version carta-md-v4.3.0 :tada:

The release is available on:

Your semantic-release bot :package::rocket: