Skip to content

Rebuild css-props page for Docusaurus V3 #41

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 9 commits into from
May 26, 2025

Conversation

didoesdigital
Copy link

@didoesdigital didoesdigital commented May 24, 2025

This PR aims to change the approach to the css-props page to avoid converting HTML to JSX and instead use HTML directly in a React page. It also updates GitHub actions and build steps.

The main commit makes these changes affecting the css-props page itself:

  • removes docs/css-props.md from .gitignore
  • imports the css-props React page into css-props.md MDX page
  • uses dangerouslySetInnerHTML to import HTML directly into React instead of using MDX to render JSX — even though it says "dangerous", the content is entirely within our control and does not include unsanitised, nefarious user-crafted HTML so it should be safe to use without risking cross-site scripting vulnerabilities
  • updates the Makefile to build the css-props page
  • moves css-props page intro from table.xsl (as XSL) to src/pages/css-props/index.js (as JSX):
    • adds JSX spaces like {" "} to ensure spaces don't collapse
    • removes a <p> wrapping a <ul>, which caused invalid nesting, which caused a "hydration error"
    • amends curly braces so they're authored like this {"{"} to avoid the content within curly braces being treated as expressions
  • adds git-tracked src/pages/css-props/_cssPropsHtml.js so we can see diffs whenever the build process changes
  • escapes octal sequence in properties.xml near -prince-text-replace e.g. changing \017F to \\017F in our source content so that it doesn't break JSX

For future content changes to properties.xml, we will need to continue to escape octal sequences. If we don't, the Docusaurus dev server will break and let us know where there are unescaped octal sequences, which is good because we won't miss any issues and we'll be able to easily find and fix them.

There are 2 giant file changes that make this PR slightly annoying to review:

  • src/pages/css-props/index.js, which deletes 20k lines of generated markup and is now replaced <div dangerouslySetInnerHTML={{ __html: cssPropsHtml }}></div>, but the new behaviour of the component is worth looking at. For review, I suggest leaving it collapsed on GitHub and just opening the new file locally to read it.
  • src/pages/css-props/_cssPropsHtml.js, which adds the generated markup. I think it's worth git-tracking this file so we can see the impact of build changes but could be persuaded otherwise. It looks a bit like this:
// NOTE: The _cssPropsHtml.js file is an automatically generated file in the
// properties/Makefile build process.
const cssPropsHtml = `
  <div id="prop-list">

    … with 20k lines here

  </div>

`;

export default cssPropsHtml;

This PR makes some other related changes:

  • updates the GitHub action workflow files to use the properties/Makefile instead of duplicating all the build steps in multiple places. This code assumes that the GitHub actions image runner has make installed. It also changes website/properties/ to src/properties/, etc.
  • removes the htmltojsx package, which is now unused
  • adds a git-tracked yarn.lock file so that every contributor to the repo has the same dependencies installed
  • makes python script files executable using chmod +x so we can remove chmod +x from GitHub Action workflows (and the properties/Makefile works locally on my machine without changes)
  • adds -p to mkdir -p output in properties/Makefile so we can run make --always-make without an error if output/ already exists
  • removes an unused module CSS style file

This PR does NOT yet include changes needed to migrate the last of the shiftWindow.js behaviour from vanilla JavaScript to modern React code to open/close CSS property sections on change of URL hash. I'd like to do that on a subsequent PR (to keep the scope of this one down) but if it's better to have the CSS props page in a more final working state, let me know.


The thinking behind the approach to using HTML (specifically: XML, XSLT, HTML, and dangerouslySetInnerHTML):

  • HTML and JSX handle whitespace differently so with content authored in HTML syntax (in the properties.xml file) and Docusaurus using React/JSX, trying to convert the HTML to JSX and preserve the spacing as intended would be tricky.
  • If we were to use XML, XSLT, HTML, htmltojsx, and JSX in MDX:
    • The formatting of the JSX tags within MDX would cause nested paragraphs (one introduced by JSX with <p> and one introduced by MDX on new line breaks), which produces in invalid HTML. That is generally not ideal and also results in Docusaurus "hydration errors" where the statically rendered markup does not match "hydrated" markup on first render in the browser.
    • All the curly braces would need to be properly escaped. Most approaches that will escape curly braces for us properly will collapse spaces incorrectly because JSX and HTML handle spaces differently e.g. Besides the standard -prince- prefix, Prince also acceptsprince- as a vendor prefix for Prince-specific CSS properties. and Also, Prince requires background-origin andbackground-clip to be adjacent.
  • If we were to use XML, and XSLT to produce JSX:
    • we'd have to either:
      • manually write XSL to produce valid JSX, including changing class to className, { to &#123;, and so on for every possible new content that might not be valid JSX, or
      • otherwise use some existing XSLT to convert XML to JSX and somehow amend it to produce the actual content we want (formatting, HTML structure, ordering, etc.) that is currently handled by our table.xsl file. I also haven't found any libraries for this that seem reliable or maintained.
    • we'd probably still have the whitespace issues noted above.

Other miscellaneous notes:

  • htmltojsx:
    • this package did not work without patching on macOS due to line ending issues so if we wanted to keep it, it would be ideal to commit a patch of it to work cross-platform for development.
    • if we were to keep the package and build on that approach somehow, we could add --silent to the yarn run htmltojsx step to avoid needing to remove text from the result in the next step of the Makefile (which incidentally also broke on mac because of sed -i differences)

This means we can remove chmod +x from GitHub Action workflows
This means we can run `make --always-make` without an error if `output/`
already exists
- removes docs/css-props.md from .gitignore
- imports css-props React page into css-props.md MDX page
- uses dangerouslySetInnerHTML to import HTML directly into React
  instead of using MDX to render JSX
- updates Makefile to build css-props page
- moves css-props page intro from table.xsl to
  src/pages/css-props/index.js as JSX, adding JSX spaces, removing a
  <p> wrapping a <ul> (invalid nesting), and escaping curly braces
- adds git-tracked src/pages/css-props/_cssPropsHtml.js so we can see
  diffs whenever the build process changes
- escapes octal sequence in properties.xml near -prince-text-replace
We no longer use htmltojsx to convert HTML to JSX and render within MDX.
Instead, we set HTML directly in a React component.

This package also did not work without patching on macOS due to line
ending issues so if we wanted to keep it, it would be ideal to commit a
patch to work cross-platform for development.
Ensure every contributor to the repo has the same dependencies installed
@mikeday
Copy link
Collaborator

mikeday commented May 26, 2025

Wow, impressive work!

I'm curious about the precise escaping we need to respect in properties.xml now, are there going to be problems with any \ character? We might want to document that in the file itself 🤔

@csant
Copy link
Contributor

csant commented May 26, 2025

Thank you, fantastic work, looks very convincing.
We can see further down the road whether we want to git track src/pages/css-props/_cssPropsHtml.js or not.
Hope the chmod changes won't be screwed up by my setup - I am mainly on Windows... But I can take care of that later, if needed.

@csant csant merged commit 072c46b into yeslogic:docusaurus-v3 May 26, 2025
@didoesdigital
Copy link
Author

@mikeday Good idea documenting the precise escaping needed in the file itself. Since the content will appear in a JavaScript template literal, escaping would be required to write a literal backslash (\\ to produce \) and any escape sequences starting with backslash that we want written as text, including hexadecimal escape sequences, unicode escape sequences and unicode code point escapes. We'd also need to escape any use of ${ that would otherwise be treated as an expression with string interpolation. (This one seems less likely to appear naturally in content about CSS.)

That said, I realised we could use String.raw on the template literal, which would reduce the amount of escaping needed to only string interpolated expressions. See new PR: #42

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants