Bug: SSR friendly adjustment

[davedbase]

@andi23rosca I was going through the process of adapting solid-site to solid-start and noticed that solid-markdown was tripping on a document reference:

ReferenceError: document is not defined
    at /Users/ddibiase/Projects/solid-site/node_modules/solid-markdown/dist/index.js:8:39186
    at /Users/ddibiase/Projects/solid-site/node_modules/solid-markdown/dist/index.js:8:109027
    at /Users/ddibiase/Projects/solid-site/node_modules/solid-markdown/dist/index.js:8:109033
    at /Users/ddibiase/Projects/solid-site/node_modules/solid-markdown/dist/index.js:1:81
    at Object.<anonymous> (/Users/ddibiase/Projects/solid-site/node_modules/solid-markdown/dist/index.js:1:293)
    at Module._compile (node:internal/modules/cjs/loader:1101:14)
    at Object.Module._extensions..js (node:internal/modules/cjs/loader:1153:10)
    at Module.load (node:internal/modules/cjs/loader:981:32)
    at Function.Module._load (node:internal/modules/cjs/loader:822:12)
    at Module.require (node:internal/modules/cjs/loader:1005:19)
error Command failed with exit code 1.
info Visit https://yarnpkg.com/en/docs/cli/run for documentation about this command.

Could we get this wrapped in an isServer check? :-)

[andi23rosca]

yes, is there a branch somewhere that has this setup with solid-start so i can test it myself?

[davedbase]

We have a somewhat-working branch here: https://github.com/solidjs/solid-site/tree/solid-start-migration/src however admittedly I have a bit more work to do on it to get it operational again. Your best bet after that is setup a basic version of solid-start and give it a spin. Using the solid-site implementation might post more headaches.

My suggestion is perhaps you can release a version that has document protected under isServer. As long as that happens I should be able to test it for you on my end.

[andi23rosca]

Yeah that was my plan but it seems like the document comes from a dependency, I don't use it in the lib :/. Weird that no one has raised the same issue in react-markdown, since the code is 80% ported from there.

[davedbase]

They're likely not doing SSR? It's a very specific use case. Even then I think some platforms have SSR friendly fallbacks. I'm not totally sure tbh, just a guess

[andi23rosca]

is the expected outcome here that SSR is working with markdown, or just that it doesn't throw an error

[davedbase]

Well as long as the library returns a proper node tree then SSR should work fine. I just skimmed what you have implemented and does seem to do that. The goal is indeed to ensure our SSR processor doesn't trip on document (because obviously it's not defined in a Node environment).

Question: is document getting called?

[andi23rosca]

I see it in the output of the bundled library but not in the source code, so I'm in the process of investigating the dependencies to see which one uses it.

[andi23rosca]

No idea where it's coming from, might actually be from Solid. Will do a proper investigation tomorrow when I have time to reproduce it locally with solid-start

[nikitavoloboev]

Any update on this? You can I think use this function from solid-primitives to make it work.

https://github.com/solidjs-community/solid-primitives/blob/main/packages/utils/src/index.ts#L27

[hyenabyte]

I have created a pull request fixing this issue here: #8 :grin:

[enpitsuLin]

8 seems can fix that, and also a workaround was use ClientOnly

[andi23rosca]

fyi everyone, did a refactor of solid-markdown recently, which resulted in v2.0.0 being published on npm there's now a ssr-demo folder in the repo to ensure the library still works in an ssr context and there's no regressions