docsify icon indicating copy to clipboard operation
docsify copied to clipboard

convert DOM creation/manipuation code to concise Solid JSX

Open trusktr opened this issue 4 years ago • 3 comments

Summary

  • This converts Docsify's internal string-based DOM code (f.e. creation of DOM elements, usages of innerHTML, etc) to Solid JSX and Solid function components. See commits for more info.
  • The goal for this PR is to be 100% backwards compatible, and this will be only an implementation detail to make Docsify's internal DOM manipulation code cleaner. Additionally, this allows the future possibility of using Solid's SSR and SSG capabilities, offloading our work to the amazing people in the Solid community who specialize in SSR/SSG.
  • Using Vue (or similar component systems with unique single-file component syntaxes) instead of Solid would require a total refactor of Docsify code, making that path much more difficult. In the changes, we can see that I merely replaced some lines of code with JSX expressions, and slightly refactored the tpl functions to be Solid component functions, making this change fairly non-invasive compared to some other options.
  • This updates Rollup and replaces Buble with Babel (we're already using Babel anyway) to add babel-preset-solid for compiling Solid JSX expressions.
  • At the moment this breaks the current SSR feature, but the intent is not to.

What kind of change does this PR introduce?

Refactor

For any code change,

  • [ ] Related documentation has been updated if needed
  • [ ] Related tests have been updated or tests have been added

Does this PR introduce a breaking change? (check one)

  • [x] Yes, but the plan is not to merge this until it does not. (Draft PR until then, or maybe never)
  • [ ] No

If yes, please describe the impact and migration path for existing applications:

Related issue, if any:

Tested in the following browsers:

  • [x] Chrome
  • [ ] Firefox
  • [ ] Safari
  • [ ] Edge
  • [ ] IE

trusktr avatar Jan 07 '22 20:01 trusktr

This pull request is being automatically deployed with Vercel (learn more).
To see the status of your deployment, click below or on the icon next to each commit.

🔍 Inspect: https://vercel.com/docsify-core/docsify-preview/GryAvaaNEjaykBNEpNqJ8wYWosZd
✅ Preview: https://docsify-preview-git-dom-with-solid-docsify-core.vercel.app

vercel[bot] avatar Jan 07 '22 20:01 vercel[bot]

This pull request is automatically built and testable in CodeSandbox.

To see build info of the built libraries, click here or the icon next to each commit SHA.

Latest deployment of this branch, based on commit 9658ed4d3a4cba036eb50f512b5c88d7c203cffd:

Sandbox Source
docsify-template Configuration

codesandbox-ci[bot] avatar Jan 07 '22 20:01 codesandbox-ci[bot]

@trusktr --

Before too much time is spent integrating SolidJS, I think it's worth getting buy-in from @docsifyjs/core.

Some maintainers have already expressed concerns and a desire to better understand the pros and cons of SolidJS vs. alternatives. To give just one example, I think it's worth considering how and why a project like Astro remains framework-agnostic while providing official support for rendering pages using React, Preact, Svelte, Alpine, Vue, SolidJS (!), and vanilla JavaScript. Given the come-and-go nature of JavaScript frameworks and, realistically, the adoption of SolidJS compared to more popular frameworks, this seems like a more future-proof approach for Docsify that still provides the SolidJS support you're hoping to achieve. We don't need to have the discussion in this PR, but it's one I believe is worth having before we green light the adoption of any framework for Docsify.

My goal is to avoid potential issues down the road if/when this work is completed and you receive pushback instead of appreciation for the efforts you've made. It's the same reason why we recommend in our PR request template that contributors create an issue, discuss, and wait for approval before creating a PR:

To avoid wasting your time, it's best to open a **feature request issue** first and wait for approval before working on it.

I also want to make sure smaller, seemingly innocuous changes aren't used as justification for larger and more significant changes down the road. I'm not saying that is what's happening here, but it's easy to imagine how "but we're already using SolidJS" could be used as an argument in future discussions.

As I've mentioned before, I have no issues with SolidJS specifically or the improvements your proposing (framework discussions aside). Maybe SolidJS is what we'll end up with. Maybe not. I just want us all to be well-informed so we can make the best decisions for Docsify long-term.

Happy to continue the discussion in a separate issue and see where we land.

jhildenbiddle avatar Feb 05 '22 21:02 jhildenbiddle