convert DOM creation/manipuation code to concise Solid JSX #1716

trusktr · 2022-01-07T20:41:49Z

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)

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:

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.

vercel · 2022-01-07T20:41:53Z

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

codesandbox-ci · 2022-01-07T20:43:34Z

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 9658ed4:

Sandbox	Source
docsify-template	Configuration

We had to pin the `colors` library (used by live-server)because the author wilfully broke it to disrupt millions of users. Marak/colors.js#285

trusktr · 2022-01-10T04:17:24Z

package.json

    "browser-sync": "^2.26.12",
    "chokidar": "^3.4.2",
+    "colors": "^1.4.0",


Pinned because live-server depends on libraries using @latest which is a good way to get breaking changes into projects, including this one: Marak/colors.js#285

trusktr · 2022-01-10T04:19:56Z

package.json

    "jest-image-snapshot": "^4.2.0",
-    "jest-playwright-preset": "^1.4.1",
+    "jest-playwright-preset": "^1.7.0",


updating Jest dependencies seems to have somehow causes jest-image-snapshot to produce slightly different font rendering, requiring an update to one of our visual snapshots.

I verified locally that both develop and this branch look identical by alternating between tabs in Chrome, and we can also verify this using Vercel's preview.

trusktr · 2022-01-10T04:22:24Z

test/integration/__snapshots__/docs.test.js.snap

@@ -1,7 +1,7 @@
 // Jest Snapshot v1, https://goo.gl/fbAQLP

 exports[`Docs Site coverpage renders and is unchanged 1`] = `
-"<section class=\\"cover show\\" style=\\"background: linear-gradient(to left bottom, hsl(127, 100%, 85%) 0%,hsl(127, 100%, 85%) 100%)\\"><div class=\\"mask\\"></div><div class=\\"cover-main\\"><p><img src=\\"http://127.0.0.1:3001/_media/icon.svg\\" data-origin=\\"_media/icon.svg\\" alt=\\"logo\\"></p><h1 id=\\"docsify-4122\\"><a href=\\"#/?id=docsify-4122\\" data-id=\\"docsify-4122\\" class=\\"anchor\\"><span>docsify <small>4.12.2</small></span></a></h1><blockquote>
+"<section class=\\"cover show\\" style=\\"\\"><div class=\\"mask\\"></div><div class=\\"cover-main\\"><p><img src=\\"http://127.0.0.1:3001/_media/icon.svg\\" data-origin=\\"_media/icon.svg\\" alt=\\"logo\\"></p><h1 id=\\"docsify-4122\\"><a href=\\"#/?id=docsify-4122\\" data-id=\\"docsify-4122\\" class=\\"anchor\\"><span>docsify <small>4.12.2</small></span></a></h1><blockquote>
 <p>A magical documentation site generator.</p></blockquote>
 <ul><li>Simple and lightweight</li><li>No statically built html files</li><li>Multiple themes</li></ul><p><a href=\\"https://github.com/docsifyjs/docsify/\\" target=\\"_blank\\" rel=\\"noopener\\">GitHub</a>
 <a href=\\"#/?id=docsify\\">Getting Started</a></p></div></section>"


As noted in some comments in the test specs (see comments containing JEST_JSDOM_BUG), this snapshot had to be updated because jsdom presents an empty style attribute despite that it is not (this is a bug in jsdom).

Generally we must avoid updating snapshots at all costs, because this can me we are allowing breaking changes into someone's CSS code of visual expectations. In this case no harm is done because I verified that the style attributes work as intended in a real browser environment, but because of jsdom's bug, I can't write a test for it in integration or unit tests. Perhaps I can write an e2e test instead, but I'm still working on fixing some e2e tests.

…with props, then use them as JSX

trusktr · 2022-01-14T19:05:40Z

packages/docsify-server-renderer/index.js

+      {config.coverpage && <Cover />}
+      <Main {...config} />
+    </>
+  );


By converting to Solid components, like this example, we will eventually be able to allow end users to provide their own component in order to override Docsify's, allowing them to make their own Docsify templates.

This will vastly improve Docsify's theme-ability: not only can they override CSS like currently (f.e. docsify-themeable being the most notable example), but they will also be able to replace the DOM structure that wraps the markdown output.

trusktr · 2022-01-14T19:07:57Z

src/core/util/polyfill/css-vars.js

@@ -26,7 +28,7 @@ export default function(color) {
      }

      get(href).then(res => {
-        const style = dom.create('style', res);
+        const style = <style>{res}</style>;


Playwright had some in-range breaking changes (regressions): microsoft/playwright#10819 and microsoft/playwright#11570 Playwright tests need to `await onceRendered()` after each route navigation to wait for render to finish before testing the state of the rendered DOM. Tests were flaky because they were looking for DOM before render finished (sometimes). Additionally, this fixes one test that was failing only locally, but not in CI, due to a RegExp check against page.url() (not sure why it would differ on CI vs local, but now the URL is explicit).

jhildenbiddle · 2022-02-05T21:53:52Z

@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.

convert some DOM creation code to Solid JSX

388ceba

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.

shorter style tag definitions

badcf91

vercel bot deployed to Preview January 7, 2022 20:48 View deployment

get tests running

5de5e5e

We had to pin the `colors` library (used by live-server)because the author wilfully broke it to disrupt millions of users. Marak/colors.js#285

vercel bot deployed to Preview January 10, 2022 03:34 View deployment

vercel bot deployed to Preview January 10, 2022 03:45 View deployment

trusktr force-pushed the dom-with-solid branch from 81dd09a to 09e3113 Compare January 10, 2022 04:01

vercel bot deployed to Preview January 10, 2022 04:01 View deployment

trusktr force-pushed the dom-with-solid branch from 09e3113 to d7c9ba0 Compare January 10, 2022 04:10

vercel bot deployed to Preview January 10, 2022 04:10 View deployment

trusktr force-pushed the dom-with-solid branch from d7c9ba0 to 26655aa Compare January 10, 2022 04:11

vercel bot deployed to Preview January 10, 2022 04:11 View deployment

update node.js versions for CI from 12,14 to 16,17

b17826f

trusktr force-pushed the dom-with-solid branch from 26655aa to b17826f Compare January 10, 2022 04:12

vercel bot deployed to Preview January 10, 2022 04:12 View deployment

update docker test script

fd494bd

trusktr commented Jan 10, 2022

View reviewed changes

vercel bot deployed to Preview January 10, 2022 04:26 View deployment

Turn the corner, cover, and main tpl functions into Solid components …

9658ed4

…with props, then use them as JSX

vercel bot deployed to Preview January 10, 2022 06:51 View deployment

trusktr commented Jan 14, 2022

View reviewed changes

vercel bot deployed to Preview January 23, 2022 08:00 View deployment

trusktr changed the title ~~convert some DOM creation code to Solid JSX~~ convert DOM creation/manipuation code to concise Solid JSX Jun 30, 2023

trusktr mentioned this pull request Jun 30, 2023

Simplify and modernize Docsify #2104

Closed

17 tasks

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

convert DOM creation/manipuation code to concise Solid JSX #1716

convert DOM creation/manipuation code to concise Solid JSX #1716

trusktr commented Jan 7, 2022 •

edited

Loading

vercel bot commented Jan 7, 2022 •

edited

Loading

codesandbox-ci bot commented Jan 7, 2022 •

edited

Loading

trusktr Jan 10, 2022

trusktr Jan 10, 2022

trusktr Jan 10, 2022 •

edited

Loading

trusktr Jan 14, 2022

trusktr Jan 14, 2022

jhildenbiddle commented Feb 5, 2022 •

edited

Loading

convert DOM creation/manipuation code to concise Solid JSX #1716

Are you sure you want to change the base?

convert DOM creation/manipuation code to concise Solid JSX #1716

Conversation

trusktr commented Jan 7, 2022 • edited Loading

Summary

What kind of change does this PR introduce?

For any code change,

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

Related issue, if any:

Tested in the following browsers:

vercel bot commented Jan 7, 2022 • edited Loading

codesandbox-ci bot commented Jan 7, 2022 • edited Loading

trusktr Jan 10, 2022

Choose a reason for hiding this comment

trusktr Jan 10, 2022

Choose a reason for hiding this comment

trusktr Jan 10, 2022 • edited Loading

Choose a reason for hiding this comment

trusktr Jan 14, 2022

Choose a reason for hiding this comment

trusktr Jan 14, 2022

Choose a reason for hiding this comment

jhildenbiddle commented Feb 5, 2022 • edited Loading

trusktr commented Jan 7, 2022 •

edited

Loading

vercel bot commented Jan 7, 2022 •

edited

Loading

codesandbox-ci bot commented Jan 7, 2022 •

edited

Loading

trusktr Jan 10, 2022 •

edited

Loading

jhildenbiddle commented Feb 5, 2022 •

edited

Loading