pngwn / MDsveX

A markdown preprocessor for Svelte.
https://mdsvex.pngwn.io
MIT License
2.44k stars 102 forks source link

Doesn't work when run from the svelte repl (or any other browser environment) #201

Open wlach opened 3 years ago

wlach commented 3 years ago

I'm trying to run mdsvex from a browser environment but it seems to be failing loading the fs module.

This trivial example fails the svelte repl:

<script>
    import { mdsvex } from "mdsvex";
    let compiled = mdsvex.compile("Hello *world*")
</script>

{compiled}

image

wlach commented 3 years ago

Poked around a little bit, it looks like there is some stuff inside mdsvex which currently is hardcoded to use fs:

https://github.com/pngwn/MDsveX/blob/cb442cab61005f454c2e724226dea30a825dd982/packages/mdsvex/src/index.ts#L153

... however I think that might be taken care of in the shims that are used to generate the list item. Should this line refer to pkg.browser instead?

https://github.com/pngwn/MDsveX/blob/cb442cab61005f454c2e724226dea30a825dd982/packages/mdsvex/rollup.config.js#L47

Even with the above fixed, I'm still seeing this error in my application:

src/main.js → public/build/bundle.js...
(!) Unresolved dependencies
https://rollupjs.org/guide/en/#warning-treating-module-as-external-dependency
mdsvex (imported by src/App.svelte, node_modules/@iridium-project/compiler/dist/main.es.js)
[!] Error: Unexpected character '�' (Note that you need plugins to import files that are not JavaScript)
node_modules/fsevents/fsevents.node (1:0)
pngwn commented 3 years ago

Yeah, the library wont run in the REPL as it is a preprocessor and designed to run in a node environment. The browser build is a bit broken at the minute, I do have plans to fix that up a little when I get the chance.

pngwn commented 3 years ago

The above code snippet you posted wouldn't actually work though as it returns an uncompiled svelte component, it would need to be processed by the compiler after being compiled by mdsvex, even if it did work correctly in the browser.

wlach commented 3 years ago

The above code snippet you posted wouldn't actually work though as it returns an uncompiled svelte component, it would need to be processed by the compiler after being compiled by mdsvex, even if it did work correctly in the browser.

Right. I was planning on using the same technique you modeled in REPLicant for that part.

I spent a bit more time working on this after my last comment, but only got a little further -- it seems there are a number of nodejs dependencies lurking in various parts of the vodebase. I may come back to this when I have some time off (this is something I really want), will file a draft PR if I get anywhere.

DougAnderson444 commented 3 years ago

Hi @pngwn and @wlach! I forked REPLicant and added mdsvex in the browser.

It works! Mostly... except code highlighting I'm a bit stumped as to why though (but thrilled about everything else!! )

The title of this issue could be changed to "Code highlighting in the browser not working"?

wlach commented 3 years ago

Nice work, all you had to do was add rollup-plugin-node-globals?

Syntax highlighting does seem to work here (in an earlier version of mdsvex which didn't have these dependencies), so it must be possible somehow:

https://mdsvex.com/playground

DougAnderson444 commented 3 years ago

Yeah, I tried but couldn't reproduce playground in the REPLicant. I think he used a custom bundle here ./public/workers/mdsvex.js since it's different from the UMD in the dist files:

https://github.com/pngwn/MDsveX/blob/95bdf67e4eafb8935e19253b679323b0b4c66b04/packages/mdsvex/rollup.config.js#L47-L49

When I tried to use the same mdsvex.js worker file as the playground in the REPL, I got a variety of problems like require not defined or import only from modules. I couldn't seem to get around it.

Maybe when pngwn see this he can give us some insight on how to make it work, because you're right, it works in the playground, somehow!

DougAnderson444 commented 3 years ago

But this issue is so close to being resolved! The only remaining item is the code highlighting.

pngwn commented 3 years ago

The short version of the whole mdsvex in the browser saga is: I created a build, copied it across to the site and then lost the source. I don't fully understand how it happened, but it did.

The adaptations for the REPL are basically as you mentioned: use a browser shim for the node globals, and use a custom load function that swaps out require for 'something else'.

The syntax highlighting is supported by copying across the language grammars, deploying them with the site, and selectively loading them in when used. They just use a different mechanism when running in the browser. There is another feature that won't work in the browser (and doesn't work in the mdsvex REPL either), that is layouts. Since they are filesystem based they don't work as is, and I haven't written a shim for them yet.

I will look at the browser build this weekend and publish a working browser version. I will also add some instruction on how to run mdsvex in the browser including syntax highlight (probably in a readme file for now).

I will be rewriting mdsvex soon, so that it does not assume any environment. This will allow it to work in the browser, node, or anything environment. I don't think we can continue to write tooling that expects a node environment, or even a traditional file system. This approach is too limiting.

DougAnderson444 commented 3 years ago

This is brilliant, thank you so much @pngwn! No environment would mean I could run mdsvex in Deno, too! Looking forward to it :)

pngwn commented 3 years ago

It is half working

I've created a working browser build with a few caveats:

There are two builds, an ESM build and a UMD build. The ESM build should be used when you want to import mdsvex or when bundling. The UMD version is good in a worker environment with importScripts as it will attach itself to the global namespace (window or worker global scope)

They need to be imported via a deep path (i haven't added export maps to mdsvex yet):

it isn't fully working

The process.browser build time replacement replacement has been broken for a while because I changed the syntax a little (in order to type the custom process object in TS), I fixed that but it isn't fully working.

The layouts are never going to work with the current implementation because they rely on a. file system and there is no way to inject a virtual FS for them to use. This will require a more significant refactor and I'll take care of it in the future. This also doesn't work in the mdsvex REPL.

The code highlighting is a bit more interesting.

This does work in the mdsvex REPL because, as @DougAnderson444 mentioned, I have a custom build. That custom build replaces requires with importScripts calls (which exists in a worker environment), when loading language metadata. However, switching between require and importScripts isn't a great solution because importScripts only exists in a worker and not in a non-worker browser environment. In a normal browser environment, dynamic import (import()) is probably a better choice. Ideally I could use dynamic imports everywhere but browser support for module imports (and dynamic imports especially) is a little patchy in workers.

The other issue is that require and importScripts are both synchronous while dynamic imports are asynchronous. This means that doing a straight swap base don some build flag, or even at runtime isn't going to work that well either. The control flow would need to be different as promises are a disease that run throughout the entirety of your program when you start using them.

if I do go down that route, I'll probably need to promisify require and importScripts for it to be consistent with import(). I could also just use the dynamic import in node, as they are well supported.

I haven't fully decided the best way to handle this. I'll probably promisify anything synchronous and add some flags/ checks to figure our the different envs, for the time being. Non-node environments will still need a custom build until I can rewrite this properly, as mdsvex uses a bunch of node built-ins.

Anyway, this should be working with the above caveats in mdsvex@0.9.5. I put together this example REPL.

pngwn commented 3 years ago

I'm going to leave this issue open until this has been addressed properly.

pngwn commented 3 years ago

The highlighter will change to shiki in v1. This has a functional browser build, I'll just need to make sure there isa. simple way to add different languages, themes, generate the correct css, support themes.

Layout loading will be the other challenge, I'll need to ensure the way layouts are accessed is injected rather than relying on the presence of a FS.