search

wooorm / parse-entities

Parse HTML character references
MIT License
49 stars 12 forks source link

SSR failure rendering markdown #18

Closed agriffis closed 3 years ago

agriffis commented 3 years ago

I'm hitting this while server-side rendering markdown via micromark in a React app:

org.graalvm.polyglot.PolyglotException: ReferenceError: document is not defined

That error is coming from this lib. Now I've found this commit:

https://github.com/wooorm/parse-entities/commit/3f830f23aa2d02042a59ebd283adf0ee121fb840

so I'm vaguely familiar with the idea that this lib provides browser-side and server-side implementations, and it's likely that I could be doing something differently in my own app that would select the right one. (Right now it's an isomorphic app but I'll probably split it into two builds to manage the size of the build for the client.)

Any hints or suggestions welcome... :pray:

wooorm commented 3 years ago

Heya!

Probably has to do with your bundler. Normally, the server-side version is used, but certain bundler pick up on the browser (or react-native) fields. So, your bundler is specifically picking an alternative version, which doesn’t work for you. And that sounds like a bug with that bundler (or a bug with how you’re using it?)

agriffis commented 3 years ago

Thanks. I'm using CRA v4 along with rescripts. I'm not sure how/where CRA sets the webpack target, looking for that now.

wooorm commented 3 years ago

I believe CRA should work. But I don’t know rescripts 🤷‍♂️

Anyway, with 26m users per month, I believe this project is doing everything okay, so it’s not something we need to change here, and thus I’ll close this!

ChristianMurphy commented 3 years ago 
org.graalvm.polyglot.PolyglotException

It's also worth noting this may be a quirk for the GraalVM JS runtime. react-markdown and remark are mostly tested on browser JS engines: V8 (chrome), SpiderMonkey (firefox), or JavaScriptCore (safari) all of which are fairly complete and stable in the JS implementation. Graal is new and Java-based, if it's anything like Java's last couple JS implementations Rhino and Nashorn, it may have an incomplete and at times non-standard implementation of the language spec.

agriffis commented 3 years ago

It's also worth noting this may be a quirk for the GraalVM JS runtime.

Unless I'm misunderstanding something, the problem is at build time. Webpack is pulling in the browser-specific implementation of this lib when it builds the app. Node would equally trip over document is not defined if it tried to SSR with the resulting bundle.

ChristianMurphy commented 3 years ago

Perhaps. :thinking: Though from a different discussion, on a similarly ployglot SSR environment with WebPack https://github.com/remarkjs/react-remark/issues/16 which also uses micromark and parse-entities under the hood, document was not an issue. :man_shrugging:

It could be the webpack config (and being more familiar with your code, your intuition may be right), I'm mostly noting that it is not the sole possibility. And that from personal experience running JS in Java (both Rhino and Nashorn), runtime/engine level bugs can confuse things, and using a browser based engine for SSR (Node, V8, Puppeteer, PlayWright, etc) where possible avoids that issue.

agriffis commented 3 years ago

Yeah, thanks for the warning, I do appreciate it. I need to use GraalJS for this, since it's fully integrated into an existing Java CMS, and needs to make transparent calls back to the Java side. On the bright side, GraalJS has been absolutely brilliant so far, much better than Rhino and Nashorn, and based on my experience, I'd recommend it as a worthy runtime.

Now that I'm looking into CRA further, it's not clear that it intends to support SSR out of the box, and I've just gotten lucky so far with isomorphic code. There are numerous articles out there about setting up the node target as a server-specific build. I don't really mind doing that—there would be other advantages for my app of splitting the builds. Right now, for example, I'm pointlessly shipping ReactDOMServer to the browser. :man_facepalming:

wooorm commented 3 years ago

Might be that you’re using CRA for something that it isn’t supposed to do 😅 Perhaps ejecting / using something else there is better suited to your goals?

ChristianMurphy commented 3 years ago

it's not clear that it intends to support SSR out of the box

It sort-of does, it can be used to do snapshotting, documented here https://create-react-app.dev/docs/pre-rendering-into-static-html-files/ But it isn't designed to do full SSR in the way Next or Gatsby do, yet (see https://github.com/facebook/create-react-app/pull/6747)

agriffis commented 3 years ago

Might be that you’re using CRA for something that it isn’t supposed to do sweat_smile Perhaps ejecting / using something else there is better suited to your goals?

Getting really close to that, especially since we're no longer even using the dev server configuration that is the crown jewel of CRA. All our dev is in Storybook, because on a deployed server, we run with umpteen roots on a single page, as the CMS structure is a mix of React-controlled and classic building blocks. It's quite... something.

agriffis commented 3 years ago

But it isn't designed to do full SSR in the way Next or Gatsby do, yet (see facebook/create-react-app#6747)

I hadn't seen that PR, and it confirms my theory. I'll be able to use the clues there, with or without ejecting (rescripts is really pretty cool), to get this working right. Thanks to you both!