zachleat / bench-framework-markdown

A set of scripts to test markdown processing speeds in various site generators/frameworks
34 stars 13 forks source link

Update Remix Benchmark #7

Closed jacob-ebey closed 2 years ago

jacob-ebey commented 2 years ago

Remix is not a static site generator and shouldn't really be a part of this benchmark, but since it is I have updated it to use https://www.npmjs.com/package/remix-ssg. This is a package that replaces the remix compiler to support SSG.

Locally this took the remix benchmark from 47.82s down to 1.08s.

Compiler support for MDX in Remix is not something we wanted to add and is by no means optimized. Moving to runtime compilation (static HTML generation time in this case) as we recommend drastically speeds things up.

zachleat commented 2 years ago

Howdy! Is this an official Remix plugin or a third party thing? I don’t see it mentioned on https://remix.run/docs/en/v1/guides/mdx

jacob-ebey commented 2 years ago

It's an unofficial 3rd party package to add SSG support for remix. It can be thought of as an attempt to add a next export concept to remix. Remix by itself is not an SSG tool and won't spit out HTML files.

zachleat commented 2 years ago

I guess I’m wary to include a special case workaround for Remix using an unofficial package?

I’m using the approach documented here https://remix.run/docs/en/v1/guides/mdx with the baseline cross-framework idea that a folder full of markdown files can be built in to a production ready project—SSG or not.

Wouldn’t the best outcome here be that the approach documented on https://remix.run/docs/en/v1/guides/mdx be improved? That’s the point of measuring things 😅

zachleat commented 2 years ago

Maybe there is a better way forward using the officially sanctioned mdx-bundler?

jacob-ebey commented 2 years ago

Revert the build / start scripts in package.json. The recommended way to do MDX should be to load it like I've done here using whatever libs you want (I used the libs here as I just copied the parsing implementation from github-md.com). Next.js supports generating routes from .md files but you are using a 3rd party to parse it in this benchmark. Seems like the same thing no?

jacob-ebey commented 2 years ago

I guess I’m wary to include a special case workaround for Remix using an unofficial package?

I’m using the approach documented here https://remix.run/docs/en/v1/guides/mdx with the baseline cross-framework idea that a folder full of markdown files can be built in to a production ready project—SSG or not.

Wouldn’t the best outcome here be that the approach documented on https://remix.run/docs/en/v1/guides/mdx be improved? That’s the point of measuring things 😅

image
ajcwebdev commented 2 years ago

If you used the approach actually documented you would have used a database, so the entire benchmark is already a workaround.

zachleat commented 2 years ago

I’m earnestly confused here @jacob-ebey, why does build-time MDX (and .md) compilation exist in Remix? Per the build performance boost of this PR specifically it would appear that the performance issue is specific to file based routing—is that accurate?

The code you’ve submitted on this PR seems to do no build-time compilation of markdown to JS by moving the markdown files out of the routes directory. This is a fine workaround but it does beg the question: why is build-time compilation happening in Remix when folks use file based routing for markdown files? That doesn’t seem to be the fault of this benchmark is it? It seems to be a bug in Remix, no? 🧐 Especially when considering the copy on the MDX docs on the Remix site: https://remix.run/docs/en/v1/guides/mdx

Isn’t the performance pitfall I stumbled into with this benchmark likely to be repeated by other folks new to Remix? It is a nice feature!

On the topic of the Next.js example and file based routing, I think you’re right about an inconsistency there. I did have a look at Next.js using File-based routing for .md files and the numbers were different! I’ll add another entry for Next.js for file-based routing.

image

I could do the same for Remix too, if you’d like me to add two project types for Remix: one with file-based routing and one without.

And finally, just to take a step back and look at the validity of the use case, a “folder of markdown files” as input is a pretty typical use case for blogs (as the other frameworks benchmarked here can attest to) and projects using a git-based CMS. I wouldn’t agree with those that think the use case is out of bounds.

jacob-ebey commented 2 years ago

I'd personally appreciate removal of the remix file based markdown routing from the benchmark, or at least a big ol' "THIS IS NOT THE PROPER WAY TO DO THIS IN REMIX" label somewhere that then points to loading the MD in a route loader. We literally say right on the docs that this is doable but not how you should approach building a blog with Remix. Here is a tutorial if you would like more reading: https://remix.run/docs/en/v1/tutorials/blog

As to why MD based routing exists in remix: the community wanted it for some reason.

Yes MDX file based routing is slow, we do not optimize for that. It's literally the last thing on my mind. If someone really needs this and wants to go against our recommendations then they can PR improvements to the compiler to optimize for it.

jacob-ebey commented 2 years ago

Also, what docs are you referring to? There is a big ol' banner on the page you keep linking to that says:

Rather than compiling your content at build-time like this document demonstrates, it's typically better UX and DX if you do this at runtime via something like mdx-bundler. It's also much more customizable and powerful. However, if you prefer to do this compilation at build-time, continue reading.

zachleat commented 2 years ago

I guess my sticking point here is that I don’t understand why file based routing of markdown and build-time compilation of markdown are coupled so tightly together in Remix. But I don’t expect you to educate me on that piece, sorry for going so deep there—I was genuinely curious.

I understand that you don’t want Remix represented on this benchmark but I sincerely do not understand the technical reason as to why existing data and results for Remix should be removed. I spent a lot of time today thinking about it, re-running benchmarks, and re-evaluating. That said I will absolutely supplement with the additional data you’ve provided in this PR and add a warning about the recommended path for Remix projects to avoid file based routing for Markdown.

But I don’t think the case has been made to remove Remix’s existing results and data, sorry. If the performance improves in the future for markdown file-based routing in Remix I will add the appropriate updates to both this repo and the blog post as well.

I know that will be frustrating to folks here and in the Remix community and I am happy to continue discussion on it.

zachleat commented 2 years ago

I’ll leave another comment here when the blog post is updated.

jacob-ebey commented 2 years ago

I guess my sticking point is "Remix isn't a generator", and including it in a "Which Generator builds Markdown the fastest?" post is misleading at best.

brophdawg11 commented 2 years ago

To emphasize @jacob-ebey's point a bit more - the blog post ("which generator...") links to a fairly exhaustive list of "site generators" in the very first paragraph. And Remix is rightfully not included in that list.

zachleat commented 2 years ago

Merged, thanks! Still working on that blog post, should be up shortly.

zachleat commented 2 years ago

Pushed a bunch of updates to https://www.zachleat.com/web/build-benchmark/

Appreciate the discussion here, y’all