Skip to content

feat: use pnpm#423

Merged
elibosley merged 3 commits intomainfrom
feat/tooling-cleanup
Mar 21, 2026
Merged

feat: use pnpm#423
elibosley merged 3 commits intomainfrom
feat/tooling-cleanup

Conversation

@elibosley
Copy link
Member

@elibosley elibosley commented Mar 20, 2026

  • also fix some SSR issues

Before Submitting This PR, Please Ensure You Have Completed The Following:

  1. Are internal links to wiki documents using relative file links?
  2. Are all new documentation files lowercase, with dash separated names (ex. unraid-os.mdx)?
  3. Are all assets (images, etc), located in an assets/ subfolder next to the .md/mdx files?
  4. Have you checked to ensure there aren't other open Pull Requests for the same update/change?
  5. Is the build succeeding?

Summary by CodeRabbit

  • New Features

    • Responsive video embed component for optimized viewing across screen sizes.
    • Variable font added and global typography enhancements.
    • Improved theme persistence and synchronization across embedded content and iframes.
  • Documentation

    • Development setup updated to use pnpm and Node 22+.
    • Embedded instructional videos standardized and updated across languages.
  • Chores

    • CI workflow and local scripts switched to pnpm; workspace and ignore rules updated.

- also fix some SSR issues
@coderabbitai
Copy link
Contributor

coderabbitai bot commented Mar 20, 2026

📝 Walkthrough

Walkthrough

Switched project tooling and docs from npm to pnpm; added a responsive ResponsiveEmbed component and styles; imported new fonts and global CSS overrides; introduced LayoutProviderWrapper; and refactored theme synchronization to use DOM data-theme, a MutationObserver, and storage events with iframe messaging.

Changes

Cohort / File(s) Summary
CI & Package Management
​.github/workflows/pr-lint.yml, .gitignore, pnpm-workspace.yaml, README.md, package.json
Switch CI/docs to pnpm (Corepack); update Node requirement to 22; setup pnpm/action-setup, change caching to pnpm, change install/lint commands to pnpm, add pnpm-debug.log* to .gitignore, and add onlyBuiltDependencies in pnpm-workspace.yaml.
ResponsiveEmbed Component & Styles
src/components/ResponsiveEmbed/index.tsx, src/components/ResponsiveEmbed/styles.module.css
Add new default-export ResponsiveEmbed component (props: src, title, optional aspectRatio, allow, referrerPolicy) and matching CSS module for responsive iframe layout.
Docs: Replace Inline iframes with Component
docs/unraid-os/.../internal-boot-faq.mdx, i18n/de/.../licensing-faq.mdx, i18n/es/.../licensing-faq.mdx, i18n/fr/.../licensing-faq.mdx, i18n/zh/.../licensing-faq.mdx
Replace hardcoded centered <iframe> blocks with ResponsiveEmbed component usages; consolidate iframe attributes into props and remove inline sizing/styling.
Global CSS & Fonts
src/css/custom.css, package.json
Import Typekit and @fontsource-variable/atkinson-hyperlegible-next; add CSS variables and typography overrides for brand font across headings, navbar, sidebar, and TOC; update scripts in package.json to use pnpm and add theme/font deps.
Layout & Theme Sync
src/theme/Layout/Provider/index.tsx, src/theme/Layout/ThemeSync.tsx, src/theme/Layout/index.tsx
Add LayoutProviderWrapper that mounts ThemeSync and IframeNavigation; rewrite ThemeSync to read/write theme via document.documentElement data-theme, use sessionStorage, emit synthetic storage events, observe DOM data-theme via MutationObserver, listen for storage events, and post messages to/from iframes; remove direct ThemeSync rendering from Layout.

Sequence Diagram(s)

sequenceDiagram
    participant User as User/Browser
    participant DOM as document.documentElement\n(data-theme)
    participant Session as sessionStorage
    participant MutationObs as MutationObserver
    participant ThemeSync as ThemeSync
    participant Iframe as Iframe(s)

    User->>DOM: set/query theme (query param / user action)
    DOM->>ThemeSync: ThemeSync reads data-theme
    ThemeSync->>Session: persist normalized theme
    ThemeSync->>Iframe: postMessage "theme-ready"

    Note over MutationObs,ThemeSync: Observe data-theme and storage changes
    User->>DOM: change data-theme attribute
    DOM->>MutationObs: mutation triggers
    MutationObs->>ThemeSync: syncThemeState()
    ThemeSync->>Session: update sessionStorage
    ThemeSync->>Iframe: postMessage "theme-changed"

    Iframe->>ThemeSync: postMessage "SET_THEME"
    ThemeSync->>DOM: apply setCurrentTheme (update data-theme)
    DOM->>MutationObs: trigger (cycle continues)
Loading

Estimated Code Review Effort

🎯 4 (Complex) | ⏱️ ~45 minutes

Poem

🐰 I hop through commits, tidy and neat,
pnpm footprints make the workflow fleet.
Videos snug in a responsive frame,
fonts crown the headings, themes play a new game.
A nibble of changes — the repo feels sweet.

🚥 Pre-merge checks | ✅ 1 | ❌ 2

❌ Failed checks (1 warning, 1 inconclusive)

Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 14.29% which is insufficient. The required threshold is 80.00%. Write docstrings for the functions missing them to satisfy the coverage threshold.
Description check ❓ Inconclusive The PR description mentions both the primary pnpm migration ('- also fix some SSR issues' indicates secondary changes) and includes the full checklist template, but the description itself is minimal and vague about the actual changes. Expand the description to clearly explain the pnpm migration scope, the SSR fixes being addressed, and which checklist items are applicable to this PR's changes.
✅ Passed checks (1 passed)
Check name Status Explanation
Title check ✅ Passed The title 'feat: use pnpm' directly addresses the primary change—migrating from npm to pnpm across workflows, scripts, documentation, and configuration files.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.

✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch feat/tooling-cleanup

Comment @coderabbitai help to get the list of available commands and usage tips.

@cloudflare-workers-and-pages
Copy link

cloudflare-workers-and-pages bot commented Mar 20, 2026

Deploying with  Cloudflare Workers  Cloudflare Workers

The latest updates on your project. Learn more about integrating Git with Workers.

Status Name Latest Commit Preview URL Updated (UTC)
✅ Deployment successful!
View logs
unraid-docs fc1817d Commit Preview URL

Branch Preview URL
Mar 20 2026, 09:52 PM

Copy link
Contributor

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 2

Caution

Some comments are outside the diff and can’t be posted inline due to platform limitations.

⚠️ Outside diff range comments (1)
.github/workflows/pr-lint.yml (1)

13-24: ⚠️ Potential issue | 🟡 Minor

Fix step ordering: pnpm must be available before actions/setup-node runs caching.

actions/setup-node@v6 with cache: pnpm requires pnpm to be available on PATH when the action executes. Currently, corepack enable runs after setup-node, causing the cache step to fail.

Move corepack enable before setup-node, or use pnpm/action-setup before setup-node (the recommended approach per setup-node docs).

🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In @.github/workflows/pr-lint.yml around lines 13 - 24, The workflow steps run
in the wrong order causing the setup-node cache to fail because pnpm isn't on
PATH; reorder the steps so "Enable Corepack" (the run: corepack enable step)
executes before the actions/setup-node@v6 step, or replace the corepack step
with the recommended pnpm/action-setup step and place that before
actions/setup-node@v6, then keep the "Install dependencies" (pnpm install
--frozen-lockfile) step after setup-node; update the job step order and
references to actions/setup-node@v6, corepack enable, pnpm/action-setup, and
pnpm install accordingly.
🧹 Nitpick comments (2)
src/theme/Layout/ThemeSync.tsx (1)

31-48: Consider using the modern StorageEvent constructor instead of deprecated API.

document.createEvent() and initStorageEvent() are deprecated. The modern approach uses the StorageEvent constructor, which is supported in all current browsers and provides cleaner code.

♻️ Suggested refactor using modern API
 function dispatchStorageChangeEvent(key: string, oldValue: string | null, newValue: string | null): void {
-  if (typeof document === "undefined" || typeof window === "undefined") {
+  if (typeof window === "undefined") {
     return;
   }
 
-  const event = document.createEvent("StorageEvent");
-  event.initStorageEvent(
-    "storage",
-    false,
-    false,
-    key,
-    oldValue,
-    newValue,
-    window.location.href,
-    window.localStorage,
-  );
+  const event = new StorageEvent("storage", {
+    key,
+    oldValue,
+    newValue,
+    url: window.location.href,
+    storageArea: window.localStorage,
+  });
   window.dispatchEvent(event);
 }
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@src/theme/Layout/ThemeSync.tsx` around lines 31 - 48, The
dispatchStorageChangeEvent function uses the deprecated
document.createEvent/initStorageEvent; replace that with the modern StorageEvent
constructor: construct a new StorageEvent("storage", { key, oldValue, newValue,
url: window.location.href, storageArea: window.localStorage, bubbles: false,
cancelable: false }) and then call window.dispatchEvent on it; keep the existing
environment guards (typeof document/window) and other parameters unchanged so
the behavior remains the same.
src/theme/Layout/Provider/index.tsx (1)

6-14: Add TypeScript types for the component props.

The children parameter lacks type annotation. For consistency with TypeScript best practices and to match the typed approach in other files (e.g., ThemeSync.tsx uses explicit ReactElement | null), consider adding proper typing.

✏️ Suggested type annotation
+import type { ReactNode } from "react";
 import React from "react";
 import LayoutProvider from "@theme-original/Layout/Provider";
 import { ThemeSync } from "../ThemeSync";
 import { IframeNavigation } from "../IframeNavigation";

-export default function LayoutProviderWrapper({ children }) {
+export default function LayoutProviderWrapper({ children }: { children: ReactNode }) {
   return (
     <LayoutProvider>
       <ThemeSync />
       <IframeNavigation />
       {children}
     </LayoutProvider>
   );
 }
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@src/theme/Layout/Provider/index.tsx` around lines 6 - 14, The
LayoutProviderWrapper component is missing TypeScript props types; add an
explicit props interface (e.g., type Props = { children?: React.ReactNode } or
use children: React.ReactElement | null to match ThemeSync) and annotate the
function signature (LayoutProviderWrapper({ children }: Props): JSX.Element or
React.ReactElement | null) so the children prop is typed and the component
return type is declared; update imports to include React types if needed and
ensure the function uses the new Props type.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.

Inline comments:
In
`@i18n/zh/docusaurus-plugin-content-docs/current/unraid-os/troubleshooting/licensing-faq.mdx`:
- Line 53: The iframe's title attribute on this Chinese (zh) MDX page is
currently English; update the iframe's title value in licensing-faq.mdx to a
Chinese/localized string (e.g., translate "Loom walkthrough" to Chinese) so
screen readers receive locale-consistent text — locate the iframe element in
this file and replace the title attribute with the localized Chinese text and
verify it matches the page locale and aria/screen-reader expectations.

In `@src/css/custom.css`:
- Around line 14-15: The Stylelint value-keyword-case error comes from unquoted,
non-generic font family identifiers in the fallback list; update the two custom
properties (--ifm-font-family-base and --ifm-heading-font-family) so that
multi-word or PascalCase family names are quoted (e.g., "BlinkMacSystemFont")
and ensure generic keywords remain lowercase (system-ui, -apple-system,
sans-serif); in short, quote any non-generic font names and keep the generic
tokens lowercase in both --ifm-font-family-base and --ifm-heading-font-family
declarations.

---

Outside diff comments:
In @.github/workflows/pr-lint.yml:
- Around line 13-24: The workflow steps run in the wrong order causing the
setup-node cache to fail because pnpm isn't on PATH; reorder the steps so
"Enable Corepack" (the run: corepack enable step) executes before the
actions/setup-node@v6 step, or replace the corepack step with the recommended
pnpm/action-setup step and place that before actions/setup-node@v6, then keep
the "Install dependencies" (pnpm install --frozen-lockfile) step after
setup-node; update the job step order and references to actions/setup-node@v6,
corepack enable, pnpm/action-setup, and pnpm install accordingly.

---

Nitpick comments:
In `@src/theme/Layout/Provider/index.tsx`:
- Around line 6-14: The LayoutProviderWrapper component is missing TypeScript
props types; add an explicit props interface (e.g., type Props = { children?:
React.ReactNode } or use children: React.ReactElement | null to match ThemeSync)
and annotate the function signature (LayoutProviderWrapper({ children }: Props):
JSX.Element or React.ReactElement | null) so the children prop is typed and the
component return type is declared; update imports to include React types if
needed and ensure the function uses the new Props type.

In `@src/theme/Layout/ThemeSync.tsx`:
- Around line 31-48: The dispatchStorageChangeEvent function uses the deprecated
document.createEvent/initStorageEvent; replace that with the modern StorageEvent
constructor: construct a new StorageEvent("storage", { key, oldValue, newValue,
url: window.location.href, storageArea: window.localStorage, bubbles: false,
cancelable: false }) and then call window.dispatchEvent on it; keep the existing
environment guards (typeof document/window) and other parameters unchanged so
the behavior remains the same.

ℹ️ Review info
⚙️ Run configuration

Configuration used: Repository UI

Review profile: CHILL

Plan: Pro

Run ID: 19e6ce38-cfe3-4397-82ed-6108be3d8856

📥 Commits

Reviewing files that changed from the base of the PR and between 5111afb and 590280f.

⛔ Files ignored due to path filters (2)
  • package-lock.json is excluded by !**/package-lock.json
  • pnpm-lock.yaml is excluded by !**/pnpm-lock.yaml
📒 Files selected for processing (16)
  • .github/workflows/pr-lint.yml
  • .gitignore
  • README.md
  • docs/unraid-os/getting-started/set-up-unraid/internal-boot-faq.mdx
  • i18n/de/docusaurus-plugin-content-docs/current/unraid-os/troubleshooting/licensing-faq.mdx
  • i18n/es/docusaurus-plugin-content-docs/current/unraid-os/troubleshooting/licensing-faq.mdx
  • i18n/fr/docusaurus-plugin-content-docs/current/unraid-os/troubleshooting/licensing-faq.mdx
  • i18n/zh/docusaurus-plugin-content-docs/current/unraid-os/troubleshooting/licensing-faq.mdx
  • package.json
  • pnpm-workspace.yaml
  • src/components/ResponsiveEmbed/index.tsx
  • src/components/ResponsiveEmbed/styles.module.css
  • src/css/custom.css
  • src/theme/Layout/Provider/index.tsx
  • src/theme/Layout/ThemeSync.tsx
  • src/theme/Layout/index.tsx

</div>
<ResponsiveEmbed
src="https://www.loom.com/embed/3ceb40440240474aaa80a0b7e3e69cb2"
title="Loom walkthrough: Unraid licensing FAQ"
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🟡 Minor

Localize the iframe title text for the zh page.

The current title is English, which is inconsistent with the page locale and screen-reader language flow.

Suggested change
-  title="Loom walkthrough: Unraid licensing FAQ"
+  title="Loom 演示:Unraid 许可证常见问题解答"
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
title="Loom walkthrough: Unraid licensing FAQ"
title="Loom 演示:Unraid 许可证常见问题解答"
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In
`@i18n/zh/docusaurus-plugin-content-docs/current/unraid-os/troubleshooting/licensing-faq.mdx`
at line 53, The iframe's title attribute on this Chinese (zh) MDX page is
currently English; update the iframe's title value in licensing-faq.mdx to a
Chinese/localized string (e.g., translate "Loom walkthrough" to Chinese) so
screen readers receive locale-consistent text — locate the iframe element in
this file and replace the title attribute with the localized Chinese text and
verify it matches the page locale and aria/screen-reader expectations.

Comment on lines +14 to +15
--ifm-font-family-base: "Figtree Variable", system-ui, -apple-system, BlinkMacSystemFont, "Segoe UI", sans-serif;
--ifm-heading-font-family: "Figtree Variable", system-ui, -apple-system, BlinkMacSystemFont, "Segoe UI", sans-serif;
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🟡 Minor

Fix the Stylelint error on the fallback font token.

This currently fails value-keyword-case on both declarations and can block lint-gated checks.

Suggested change
-  --ifm-font-family-base: "Figtree Variable", system-ui, -apple-system, BlinkMacSystemFont, "Segoe UI", sans-serif;
-  --ifm-heading-font-family: "Figtree Variable", system-ui, -apple-system, BlinkMacSystemFont, "Segoe UI", sans-serif;
+  --ifm-font-family-base: "Figtree Variable", system-ui, -apple-system, "Segoe UI", sans-serif;
+  --ifm-heading-font-family: "Figtree Variable", system-ui, -apple-system, "Segoe UI", sans-serif;
🧰 Tools
🪛 Stylelint (17.4.0)

[error] 14-14: Expected "BlinkMacSystemFont" to be "blinkmacsystemfont" (value-keyword-case)

(value-keyword-case)


[error] 15-15: Expected "BlinkMacSystemFont" to be "blinkmacsystemfont" (value-keyword-case)

(value-keyword-case)

🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@src/css/custom.css` around lines 14 - 15, The Stylelint value-keyword-case
error comes from unquoted, non-generic font family identifiers in the fallback
list; update the two custom properties (--ifm-font-family-base and
--ifm-heading-font-family) so that multi-word or PascalCase family names are
quoted (e.g., "BlinkMacSystemFont") and ensure generic keywords remain lowercase
(system-ui, -apple-system, sans-serif); in short, quote any non-generic font
names and keep the generic tokens lowercase in both --ifm-font-family-base and
--ifm-heading-font-family declarations.

Copy link
Contributor

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🧹 Nitpick comments (1)
src/css/custom.css (1)

1-2: Consider string notation for the Typekit import to satisfy Stylelint.

Stylelint's import-notation rule expects string notation. If this blocks CI, update the import:

-@import url("https://use.typekit.net/ymm4lqb.css");
+@import "https://use.typekit.net/ymm4lqb.css";

Both are functionally equivalent.

🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@src/css/custom.css` around lines 1 - 2, Stylelint wants string notation for
imports: replace the Typekit line `@import`
url("https://use.typekit.net/ymm4lqb.css"); with string notation `@import`
"https://use.typekit.net/ymm4lqb.css"; (leave the `@import`
"@fontsource-variable/atkinson-hyperlegible-next"; line as-is) so the rule
`import-notation` is satisfied.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.

Nitpick comments:
In `@src/css/custom.css`:
- Around line 1-2: Stylelint wants string notation for imports: replace the
Typekit line `@import` url("https://use.typekit.net/ymm4lqb.css"); with string
notation `@import` "https://use.typekit.net/ymm4lqb.css"; (leave the `@import`
"@fontsource-variable/atkinson-hyperlegible-next"; line as-is) so the rule
`import-notation` is satisfied.

ℹ️ Review info
⚙️ Run configuration

Configuration used: Repository UI

Review profile: CHILL

Plan: Pro

Run ID: 0cfa92d4-11c3-4de5-9b68-6e538b125a06

📥 Commits

Reviewing files that changed from the base of the PR and between 590280f and f1f95e3.

⛔ Files ignored due to path filters (1)
  • pnpm-lock.yaml is excluded by !**/pnpm-lock.yaml
📒 Files selected for processing (3)
  • .github/workflows/pr-lint.yml
  • package.json
  • src/css/custom.css
✅ Files skipped from review due to trivial changes (1)
  • .github/workflows/pr-lint.yml

Copy link
Contributor

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 1

♻️ Duplicate comments (1)
src/css/custom.css (1)

15-16: ⚠️ Potential issue | 🟠 Major

Resolve remaining value-keyword-case violations in font fallbacks and text-rendering values.

Line 16 repeats the previously flagged fallback-font casing issue, and Lines 44/88 introduce the same rule violation for optimizeLegibility.

Suggested fix
-  --unraid-brand-font-family: "nudista-web", -apple-system, system-ui, "Segoe UI", Roboto, Helvetica, Arial, sans-serif;
-  --ifm-font-family-base: "Atkinson Hyperlegible Next Variable", system-ui, -apple-system, BlinkMacSystemFont, "Segoe UI", sans-serif;
+  --unraid-brand-font-family: "nudista-web", -apple-system, system-ui, "Segoe UI", "Roboto", "Helvetica", "Arial", sans-serif;
+  --ifm-font-family-base: "Atkinson Hyperlegible Next Variable", system-ui, -apple-system, "BlinkMacSystemFont", "Segoe UI", sans-serif;
@@
-  text-rendering: optimizeLegibility;
+  text-rendering: optimizelegibility;
@@
-  text-rendering: optimizeLegibility;
+  text-rendering: optimizelegibility;

Also applies to: 44-44, 88-88

🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@src/css/custom.css` around lines 15 - 16, Normalize the value keyword casing:
update the font-family fallbacks in --unraid-brand-font-family and
--ifm-font-family-base so all generic/system keywords are lowercase (e.g.,
system-ui, -apple-system, sans-serif) and adjust any text-rendering declarations
(the text-rendering property occurrences) to use the lowercase keyword variant
expected by the linter (e.g., optimizelegibility if your lint rule enforces
lowercase). Ensure only the generic keywords are lowercased and quoted font
names keep their original capitalization/quotes.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.

Inline comments:
In `@src/css/custom.css`:
- Line 1: Replace the `@import` url(...) statement in custom.css with string
import notation to satisfy the Stylelint import-notation rule: locate the
`@import` line (the statement referencing "https://use.typekit.net/ymm4lqb.css")
and change it to use a quoted string form (e.g., `@import`
"https://use.typekit.net/ymm4lqb.css";) so the linter accepts the import.

---

Duplicate comments:
In `@src/css/custom.css`:
- Around line 15-16: Normalize the value keyword casing: update the font-family
fallbacks in --unraid-brand-font-family and --ifm-font-family-base so all
generic/system keywords are lowercase (e.g., system-ui, -apple-system,
sans-serif) and adjust any text-rendering declarations (the text-rendering
property occurrences) to use the lowercase keyword variant expected by the
linter (e.g., optimizelegibility if your lint rule enforces lowercase). Ensure
only the generic keywords are lowercased and quoted font names keep their
original capitalization/quotes.

ℹ️ Review info
⚙️ Run configuration

Configuration used: Repository UI

Review profile: CHILL

Plan: Pro

Run ID: b9d9eac7-814e-468c-b9e9-d19484f42f63

📥 Commits

Reviewing files that changed from the base of the PR and between f1f95e3 and fc1817d.

📒 Files selected for processing (1)
  • src/css/custom.css

@elibosley elibosley added this pull request to the merge queue Mar 21, 2026
Merged via the queue into main with commit 5623f69 Mar 21, 2026
3 checks passed
@elibosley elibosley deleted the feat/tooling-cleanup branch March 21, 2026 15:37
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.

1 participant