Conversation
| sidebar_label: Scalability | ||
| --- | ||
|
|
||
| import MyPartial from "@site/src/components/Shared/kratos/10_scalability.mdx" |
There was a problem hiding this comment.
The Kratos Courier content is not applicable to Hydra.
There was a problem hiding this comment.
This is existing content that has a high level 'scalability' section that mentions Hydra. Apart from the Mail Courier sub-section, does the high level scalability sections still apply to Hydra?
|
This redesign mentions Polis in the context of SCIM and Enterprise SSO many times. Polis only does SAML, nothing else. REPLY: Please refer to Deepak, I'm 99.999999999% sure Polis support our SCIM functionality. See https://www.ory.com/polis |
|
It is still very confusing for me to find concrete pages. Like for example Hydra Guides. If I start on the OEL page (https://docs-7qgoc8ot0-ory.vercel.app/docs/oel/getting-started) and click Hydra in the sidebar, I have an option "Guides". When I click that, I get a selection of "Running Hydra in Docker" and a couple others, but few details. Whereas when I start on the Ory Network page (https://docs-7qgoc8ot0-ory.vercel.app/docs/network/getting-started) and click on Hydra in the Sidebar, I also see an option Guides. But that link leads me to a completely different set of Hydra Guides which actually explain how to use the product. REPLY: The focus of this round is to create a separate framework for the different deployments. That is why Network has far more content. We have yet to write stuff for OEL. We reorganized the content and will start creating (or reusing/sharing) content where appropriate. |
|
Method: I (with Claude) diffed every Pages dropped from navigationDoc ids present in the old
Pages correctly present in
|
All of these have redirects in the vercel.json file. What are you doing to generate these as errors/404s etc? |
|
@coderabbitai review again |
|
✅ Actions performedReview triggered.
|
|
Caution Failed to replace (edit) comment. This is likely due to insufficient permissions or the comment being deleted. Error details |
There was a problem hiding this comment.
Actionable comments posted: 4
Note
Due to the large number of review comments, Critical severity comments were prioritized as inline comments.
♻️ Duplicate comments (1)
docusaurus.config.ts (1)
19-20:⚠️ Potential issue | 🟠 MajorKeep broken-link failures enabled in CI.
Line 19 and Line 20 remove the one automated check most likely to catch the navigation regressions this redesign is introducing. If CI uses the same config, broken links will ship silently. Gate this on
process.env.CIinstead of relaxing it globally.Suggested change
- onBrokenLinks: "warn", - onBrokenMarkdownLinks: "warn", + onBrokenLinks: process.env.CI ? "throw" : "warn", + onBrokenMarkdownLinks: process.env.CI ? "throw" : "warn",Verify whether CI shares this config without an override. Expected result: CI builds should still fail on broken links even if local rebuilds only warn.
#!/bin/bash set -euo pipefail echo "Current broken-link policy in docusaurus.config.ts:" sed -n '17,22p' docusaurus.config.ts echo echo "Any env-specific override already present?" rg -n -C2 'onBrokenLinks|onBrokenMarkdownLinks|process\.env\.CI|CI' docusaurus.config.ts || true echo echo "Build commands in package scripts:" if [ -f package.json ]; then rg -n -C2 'docusaurus build|npm run build|pnpm build|yarn build' package.json || true fi echo echo "Build commands in GitHub workflows:" if [ -d .github ]; then rg -n -C2 'docusaurus build|npm run build|pnpm build|yarn build' .github || true fi🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@docusaurus.config.ts` around lines 19 - 20, The config currently sets onBrokenLinks and onBrokenMarkdownLinks to "warn" globally; change this so CI still fails: update the values for onBrokenLinks and onBrokenMarkdownLinks in docusaurus.config.ts to use a conditional based on process.env.CI (e.g., use "error" when process.env.CI is truthy, otherwise "warn") so local builds warn but CI builds fail; locate the properties named onBrokenLinks and onBrokenMarkdownLinks in the exported config object and replace the static "warn" strings with the CI-gated expression.
🟠 Major comments (19)
.coderabbit.yaml-4-5 (1)
4-5:⚠️ Potential issue | 🟠 MajorGlobal MDX exclusion removes review coverage from the docs surface.
Lines 4–5 exclude every
*.mdxfile, which suppresses review on the main docs content in a docs-focused repo/PR. This can hide navigation and link regressions.Suggested adjustment
reviews: review_status: false - # Temporarily exclude MDX files from review processing. + # Keep MDX in scope; only exclude generated/vendor artifacts if needed. path_filters: - - "!**/*.mdx" + - "!**/node_modules/**" + - "!**/build/**" + - "!**/.docusaurus/**"🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In @.coderabbit.yaml around lines 4 - 5, The global path_filters entry currently negates all MDX files via the pattern "!**/*.mdx", which removes review coverage for docs; update the .coderabbit.yaml by removing or narrowing that exclusion (e.g., delete the "!**/*.mdx" pattern or replace it with more specific exclusions such as "!public/**.mdx" or an allow-list for your docs directory) so that documentation MDX files are included in reviews; look for the path_filters key and the pattern "!**/*.mdx" to make the change.src/pages/_assets/product-selector-graph.module.css-25-25 (1)
25-25:⚠️ Potential issue | 🟠 MajorConfigure stylelint to allow CSS Modules
:global()pseudo-class.The file uses
:global()selectors extensively (lines 25, 34, 41, 46–252) to style global Mermaid classes within the CSS Module. The stylelint configuration does not currently ignore theglobalpseudo-class, causingselector-pseudo-class-no-unknownto treat it as invalid and block CI.Add the following to
.stylelintrc.json:"selector-pseudo-class-no-unknown": [ true, { "ignorePseudoClasses": ["global"] } ]🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/pages/_assets/product-selector-graph.module.css` at line 25, The Stylelint rule is flagging the CSS Modules :global() pseudo-class used in selectors like ".graphContainer :global(.mermaid) :global(.cluster)"; update the Stylelint config (in .stylelintrc.json) to allow the "global" pseudo-class by adding "global" to the selector-pseudo-class-no-unknown ignorePseudoClasses array so stylelint no longer treats :global as unknown.src/pages/_assets/product-selector-graph.module.css-142-145 (1)
142-145:⚠️ Potential issue | 🟠 MajorRemove redundant
backgroundshorthand declarations that conflict withbackground-colorlonghand.The
declaration-block-no-shorthand-property-overridesrule (enabled viastylelint-config-standard-scss) flags when a shorthand property overrides a longhand property in the same declaration block. The redundantbackground: transparent !important;should be removed from all four affected locations, keeping onlybackground-colorfor clarity.🩹 Proposed fix
.graphContainer :global(.mermaid) :global(.edgeLabel) { background-color: transparent !important; - background: transparent !important; color: var(--ifm-color-emphasis-800) !important; font-size: 11px !important; font-weight: 600 !important; @@ .graphContainer :global(.mermaid) :global(.edgeLabel) :global(foreignObject) :global(div) { background-color: transparent !important; - background: transparent !important; } @@ :global([data-theme="dark"]) .graphContainer :global(.mermaid) :global(.edgeLabel) { background-color: transparent !important; - background: transparent !important; color: var(--ifm-color-emphasis-100) !important; border: none !important; } @@ :global([data-theme="dark"]) .graphContainer :global(.mermaid) :global(.edgeLabel) :global(foreignObject) :global(div) { background-color: transparent !important; - background: transparent !important; }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/pages/_assets/product-selector-graph.module.css` around lines 142 - 145, Remove the redundant background shorthand that overrides the longhand background-color in the CSS rule for .graphContainer :global(.mermaid) :global(.edgeLabel) (and the three other similar selectors flagged in this file); keep only background-color: transparent !important; and color as-is so the declaration-block-no-shorthand-property-overrides lint rule is satisfied and the longhand value is not shadowed by a subsequent background shorthand.docs/products/product-arch-28-57 (1)
28-57:⚠️ Potential issue | 🟠 MajorMermaid diagram won’t render without a fenced block.
Line 28 starts Mermaid syntax directly, but it must be wrapped in a ```mermaid code fence to render.
Proposed fix
-flowchart TD +```mermaid +flowchart TD Start([What are you building?]) --> Q1{Need to manage<br>user identities?} Q1 -->|Yes| Kratos[✓ Kratos] @@ Q5 -->|Yes| Oathkeeper[✓ Add Oathkeeper] Q5 -->|No| Done([Your stack is ready]) Oathkeeper --> Done +```🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@docs/products/product-arch` around lines 28 - 57, The Mermaid diagram starting with "flowchart TD" (nodes like Start, Q1, Kratos, Polis, Hydra, Keto, Oathkeeper, Done) must be wrapped in a fenced Mermaid code block; update the doc so the entire diagram is enclosed between a starting ```mermaid line and a closing ``` line (no other changes to node names or arrows), ensuring the exact sequence from "flowchart TD" through "Oathkeeper --> Done" sits inside that fence so the renderer can parse it.docs/oss/index.md-1-4 (1)
1-4:⚠️ Potential issue | 🟠 MajorAvoid shipping an empty OSS landing page.
With
hide_title: trueon Line 3 and no body content, this route renders as a blank page. Add at least a short intro (or removehide_title) so/docs/ossis navigable.Proposed fix
--- title: Ory Open Source hide_title: true --- + +Welcome to Ory Open Source docs. Start with deployment, configuration, and product guides tailored for OSS users.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@docs/oss/index.md` around lines 1 - 4, The OSS docs page currently contains only frontmatter with hide_title: true which results in a blank route; either add a short introductory paragraph below the YAML frontmatter in docs/oss/index.md (e.g., a one- or two-sentence intro about the Ory open source offering) or remove the hide_title: true key so the title is shown—update the file so /docs/oss renders non-empty content.src/components/Shared/kratos/e2e-integration-tests.tsx-18-41 (1)
18-41:⚠️ Potential issue | 🟠 MajorDon't render a self-hosted setup example for the
networkproduct.Lines 18–41 set
commandPrefixto"ory"for Ory Network and"kratos"for self-hosted deployments. However, the surrounding text (lines 35–36) describes booting Ory Kratos locally withDSN=memoryand a config file—a self-hosted setup that is incorrect for Ory Network, which is a managed service. Line 40 will produceory serve -c ./path/to/config/kratos.yml --devfor thenetworkbranch, which is not a valid command pattern.Additionally, line 76 contains a hardcoded
hydra servecommand (should bekratos serveor use thecommandPrefixvariable like line 40 does).Either restrict this entire section to
oel | ossproducts, or provide separate Network-specific instructions that don't assume self-hosted operation.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/components/Shared/kratos/e2e-integration-tests.tsx` around lines 18 - 41, The text and example command incorrectly present a self-hosted Kratos startup for the "network" product by using commandPrefix (set to "ory" for network) and a hardcoded "hydra serve" elsewhere; update the component so this section only renders for self-hosted products (check product values "oel" | "oss") or split into two branches: for self-hosted use the existing DSN=memory ${commandPrefix} serve example (with commandPrefix set to "kratos") and for Network render an explicit note and Network-specific instructions (no local serve command). Also replace the hardcoded "hydra serve" occurrence to use the commandPrefix variable or the correct "kratos serve" in the self-hosted branch and ensure the UI text never suggests running a local Kratos server when product === "network".src/components/QuickStarts/hooks/useClickOutside.ts-3-6 (1)
3-6:⚠️ Potential issue | 🟠 MajorAccept nullable refs from callers.
Line 4 is typed as
RefObject<HTMLElement>, but actual callers inDeploymentModeSelector.tsxandLanguageFilter.tsxuseuseRef<HTMLDivElement | null>(null), which yieldsRefObject<HTMLDivElement | null>. This causes a type mismatch. The hook implementation already safely handles null refs via the guard clauseif (ref.current && ...)on line 14, but the signature should reflect this. Make the hook generic to accept nullable refs.🛠️ Proposed fix
-export function useClickOutside( - ref: RefObject<HTMLElement>, +export function useClickOutside<T extends HTMLElement>( + ref: RefObject<T | null>, isOpen: boolean, onClose: () => void, ) {🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/components/QuickStarts/hooks/useClickOutside.ts` around lines 3 - 6, The hook signature for useClickOutside currently requires RefObject<HTMLElement> but callers pass nullable refs; update the signature to accept nullable refs by making the hook generic (e.g., useClickOutside<T extends HTMLElement | null>) and change the ref parameter type to RefObject<T> (or RefObject<T | null>) so callers using useRef<HTMLDivElement | null>(null) match; keep the existing null-guard logic inside the function and ensure the generic is used in the function definition and any exported typing to preserve callers' inferred element type.src/components/Shared/kratos/e2e-integration-tests.tsx-66-77 (1)
66-77:⚠️ Potential issue | 🟠 MajorFix the command snippet and update prose for environment variable value.
The code block has multiple issues:
- Uses
hydra serveinstead ofkratos serve- Stray
)at end of command- Indentation breaks shell syntax
- Prose contradicts code by stating
DEV_DISABLE_API_FLOW_ENFORCEMENT=1, but per Ory documentation, boolean environment variables require=true(or=false), not=1Update both the prose and code block:
🛠️ Proposed fix
<p> If you want to test React Native (rendered as a web application) in Cypress, you need to disable security features preventing browser from executing self-service API flows. To do this, set the environment - variable <code>DEV_DISABLE_API_FLOW_ENFORCEMENT=1</code>: + variable <code>DEV_DISABLE_API_FLOW_ENFORCEMENT=true</code>: </p> <CodeBlock language="sh"> {`DSN=memory \\ - DEV_DISABLE_API_FLOW_ENFORCEMENT=true - hydra serve -c ./path/to/config/kratos.yml --dev > "kratos.e2e.log" 2>&1 &)`} +DEV_DISABLE_API_FLOW_ENFORCEMENT=true \\ +kratos serve -c ./path/to/config/kratos.yml --dev > "kratos.e2e.log" 2>&1 &`} </CodeBlock>🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/components/Shared/kratos/e2e-integration-tests.tsx` around lines 66 - 77, Update the prose to say DEV_DISABLE_API_FLOW_ENFORCEMENT=true (not =1) and fix the shell snippet inside the CodeBlock: replace the incorrect "hydra serve" with "kratos serve", remove the stray trailing ")" and correct line breaks/indentation so environment variables (DSN=memory and DEV_DISABLE_API_FLOW_ENFORCEMENT=true) are exported/set before invoking kratos, and ensure the kratos serve command uses the correct flags and backgrounding/redirect syntax (e.g., proper use of & and redirections) so the snippet is a valid shell command; locate and edit the CodeBlock component and surrounding explanatory paragraph in kratos/e2e-integration-tests.tsx.src/components/OryNetworkCta/ory-network-cta.tsx-17-21 (1)
17-21:⚠️ Potential issue | 🟠 MajorDon't call
useBaseUrl()conditionally.Lines 17-21 invoke
useBaseUrl()only when the imported asset is not a string. SinceuseBaseUrlis a React hook, this violates the Rules of Hooks—hooks must always be called unconditionally at the top level of a component. Move theuseBaseUrl()call outside the conditional and reuse the result.🛠️ Proposed fix
export const OryNetworkCta = () => { const { title, description, ctaLabel, href } = CTA_CONFIG + const fallbackImageSrc = useBaseUrl("/img/network-cta/network.png") const imageSrc = typeof networkImg === "string" ? networkImg - : (networkImg as { default?: string }).default ?? - useBaseUrl("/img/network-cta/network.png") + : (networkImg as { default?: string }).default ?? fallbackImageSrc🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/components/OryNetworkCta/ory-network-cta.tsx` around lines 17 - 21, The component currently calls useBaseUrl() conditionally inside the imageSrc expression which breaks the Rules of Hooks; call useBaseUrl once at the top of the component (e.g., const baseNetworkImg = useBaseUrl("/img/network-cta/network.png")) and then compute imageSrc using networkImg and baseNetworkImg (use typeof networkImg === "string" ? networkImg : (networkImg as { default?: string }).default ?? baseNetworkImg) so useBaseUrl is invoked unconditionally and imageSrc logic is unchanged.src/components/Shared/self-hosted/deployment.tsx-172-179 (1)
172-179:⚠️ Potential issue | 🟠 MajorThis shared section hard-codes OEL-only navigation into every deployment page.
DeploymentDatabaseis shared acrossnetwork,oel, andoss, but this block always renders OEL-specific copy../oel/high-performance-poolingresolves relative to the current page, and the#high-performance-poolingjump on Line 200 has no matchingidon Line 172. Gate this section byproduct(or compute a deployment-aware destination) and add an explicit heading id.Also applies to: 195-204
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/components/Shared/self-hosted/deployment.tsx` around lines 172 - 179, The shared DeploymentDatabase component currently renders OEL-specific content unconditionally and uses a relative ./oel/high-performance-pooling link plus a mismatched anchor; update DeploymentDatabase to only render the "High-performance pooling" block when product === "oel" (or compute a deployment-aware destination based on the current page context), change the link to an explicit deployment-aware path rather than a relative "./oel/..." URL, and add a matching id attribute (e.g., id="high-performance-pooling") to the <h5> heading so the `#high-performance-pooling` anchor works; ensure the same gating and id fix is applied to the other repeated block around lines 195-204.sidebars-quickstarts.ts-52-58 (1)
52-58:⚠️ Potential issue | 🟠 MajorDon't send the Network sidebar into OEL docs.
This
Ory NetworkPolis section mixes a Network landing page with an OEL quickstart. Following that second item switches deployment model from inside the Network sidebar and undermines the split-navigation structure this PR introduces. Keep this section network-only, or drop the second entry until a Network-specific quickstart exists.🧭 Suggested fix
items: [ "network/polis/quickstarts/index", - "oel/polis/quickstarts/quickstart", ],🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@sidebars-quickstarts.ts` around lines 52 - 58, The "Enterprise SSO AuthZ" Network sidebar currently mixes Network and OEL content by including "oel/polis/quickstarts/quickstart"; remove that entry from the items array (leave only "network/polis/quickstarts/index") so the Network sidebar remains network-only, or alternatively move the "oel/polis/quickstarts/quickstart" entry into the appropriate OEL sidebar instead; update the items list in sidebars-quickstarts.ts for the "Enterprise SSO AuthZ" category accordingly.src/components/welcomePage/ContentSection.tsx-10-15 (1)
10-15:⚠️ Potential issue | 🟠 MajorUse a stable Console signup URL here.
The
flow=UUID is transient state. Baking it into a homepage CTA means this card will eventually send users to an expired registration flow instead of starting signup. Point this at a stable Console entry URL and let Console mint a fresh flow.🔗 Suggested fix
- to: "https://console.ory.sh/registration?flow=e27eca2b-ca8a-4722-8a4e-9c52a20aa0d4", + to: "https://console.ory.sh/registration",🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/components/welcomePage/ContentSection.tsx` around lines 10 - 15, The "Ory Console" card uses a transient registration flow URL (the `to` property containing `flow=<UUID>`), which will expire; update the `to` value in the object with label "Ory Console" inside ContentSection (the card definition) to a stable Console entry URL (e.g., remove the `flow=` query and point to a persistent entry like https://console.ory.sh/registration or https://console.ory.sh/) so Console can mint a fresh flow when users click the CTA.sidebars.ts-7-10 (1)
7-10:⚠️ Potential issue | 🟠 MajorAvoid importing Docusaurus internal
srctypes.Line 7 imports from
@docusaurus/plugin-content-docs/src/sidebars/types, which is an internal module and not supported for public use. This can break on dependency updates. Use the publicSidebarsConfigAPI instead.🔧 Proposed refactor (public API only)
import type { SidebarsConfig } from "@docusaurus/plugin-content-docs" - -import { SidebarItemConfig } from "@docusaurus/plugin-content-docs/src/sidebars/types" - -type SidebarItemsConfig = SidebarItemConfig[] +type SidebarItemsConfig = SidebarsConfig[string]🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@sidebars.ts` around lines 7 - 10, Replace the internal import of SidebarItemConfig with the public SidebarsConfig API: remove the import from "@docusaurus/plugin-content-docs/src/sidebars/types" and instead import SidebarsConfig from the public package (e.g., "@docusaurus/plugin-content-docs"), then update the type alias SidebarItemsConfig to use the public SidebarsConfig type (or derive the specific item type from SidebarsConfig if you need a narrower shape) so the file no longer depends on internal src types.sidebars-oss.ts-55-60 (1)
55-60:⚠️ Potential issue | 🟠 MajorKeep the OSS sidebar inside OSS/shared docs.
These entries live under “Ory Open Source” but send users to
oel/...andself-hosted/oel/...pages. That makes the new deployment split feel broken as soon as someone clicks into a product from the OSS nav.Also applies to: 134-142, 205-211, 251-255, 427-430
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@sidebars-oss.ts` around lines 55 - 60, The OSS sidebar currently contains entries that reference OEL pages ("oel/kratos/intro", "kratos/install", "self-hosted/oel/kratos/upgrade", "self-hosted/oel/kratos/changelog", "oel/kratos/quickstart") which breaks the OSS/shared split; move or replace these entries so the OSS sidebar only links to OSS/shared docs — either remove them from the OSS sidebar array and add equivalent links to the OEL/self-hosted sidebar, or update the ids to the correct OSS/shared ids, and apply the same fix for the other occurrences noted (the other blocks that reference "oel/..." or "self-hosted/oel/..."). Ensure the references in the sidebar data structure (the items with type: "ref" and string IDs) point to the proper namespace (OSS/shared) or are relocated to the OEL sidebar.src/navbar.ts-30-31 (1)
30-31:⚠️ Potential issue | 🟠 MajorMake the Products dropdown target absolute.
Unlike the other top-level dropdowns, this
tovalue is relative. From a nested docs page it will resolve against the current URL instead of/docs/products/products-overview.Suggested fix
- to: "products/products-overview", + to: "/docs/products/products-overview",🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/navbar.ts` around lines 30 - 31, The Products dropdown entry currently uses a relative `to` value ("products/products-overview") which can resolve incorrectly from nested docs pages; update the dropdown item (the object with type: "dropdown" and its `to` property in src/navbar.ts) to use the absolute docs path (e.g. "/docs/products/products-overview") consistent with the other top-level dropdowns so it always targets the correct top-level products overview page.src/components/QuickStarts/quickstart-filter.tsx-107-124 (1)
107-124:⚠️ Potential issue | 🟠 MajorClamp
languageto the current category.The effect mirrors any
?language=value into state without checkingavailableLanguages. After a category/deployment change, a stale language can blank outfilteredByLanguage, and if the new category only has one language the picker disappears so the user can't recover from the empty state.Also applies to: 137-141
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/components/QuickStarts/quickstart-filter.tsx` around lines 107 - 124, The effect handling URL params sets activeLanguage directly from getSearchParams without validating it against the selected category's availableLanguages, which can leave activeLanguage stale/invalid after category changes; update the useEffect (the hook that reads location.search) to clamp the urlLanguage to the current category by checking visibleCategories / the chosen category's availableLanguages (or a derived availableLanguages array) before calling setActiveLanguage, and if the urlLanguage is not in that list fall back to a valid language (e.g., the category default or "all"); similarly ensure the same validation logic is applied in the other effect referenced (lines 137-141) and keep languageFilterRef.current?.close(), setActiveCategoryId, and setActiveLanguage calls intact but only after computing the validated language.src/components/welcomePage/SelectFrameworkBlock.tsx-30-37 (1)
30-37:⚠️ Potential issue | 🟠 MajorUse a real default selection here.
Falling back to
frameworks[1]makes the snippet/guide render from the second option while no tab is active, and it crashes when only one framework is passed. Default toframeworks[0]and guard the empty-array case.Suggested fix
- const selected = - frameworks.find((f) => f.value === selectedFramework) ?? frameworks[1] + const selected = + frameworks.find((f) => f.value === selectedFramework) ?? frameworks[0] + if (!selected) { + return null + }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/components/welcomePage/SelectFrameworkBlock.tsx` around lines 30 - 37, The current fallback uses frameworks[1] which can render the wrong item or crash when frameworks has length < 2; change the selection fallback to use frameworks[0] and guard an empty array: in SelectFrameworkBlock replace the selected assignment that references frameworks[1] with one that falls back to frameworks[0] and then to a safe default object (e.g., { value: "", snippet: "" }) when frameworks is empty, so selected, snippetPrefix, and snippetRest never access undefined; keep the existing snippetPrefixes/snippetPrefix/snippetRest logic but ensure they read from the safe selected.snippet.src/components/OryArchitectureDiagram.tsx-36-47 (1)
36-47:⚠️ Potential issue | 🟠 MajorAdd
identityAnswertoFitViewOnChangedependency and lower theminZoomclamp.
identityAnswercan add/remove the external IdP and switch the graph between compact and multi-row layouts, butFitViewOnChangeonly watchesselectedProducts. The hardminZoom: 1clamp also prevents the wider layout from fitting on narrow viewports.Include
identityAnswerin the dependency array and pass it toFitViewOnChangeat line 231. Review and likely lower theminZoomvalue across allfitViewcalls.Also applies to: 72–74, 226–227
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/components/OryArchitectureDiagram.tsx` around lines 36 - 47, FitViewOnChange only depends on selectedProducts but must also depend on identityAnswer because adding/removing the external IdP changes layout; update the dependency array in FitViewOnChange to include identityAnswer and update any call sites that render/use <FitViewOnChange ...> (pass the identityAnswer prop where FitViewOnChange is instantiated), and relax the hard minZoom clamp (lower from 1 to a smaller value such as 0.5) in all fitView({ padding..., duration..., minZoom }) invocations so wider layouts can fit on narrow viewports; locate references by the FitViewOnChange function name, the identityAnswer variable, and fitView from useReactFlow.src/components/OryArchitectureDiagram.tsx-191-198 (1)
191-198:⚠️ Potential issue | 🟠 MajorGuard the fullscreen API before chaining
.then().
requestFullscreen?.()/exitFullscreen?.()can returnundefined, and both promises can reject. On unsupported or denied browsers this turns the fullscreen button into a runtime error path. The optional chaining?.()evaluates toundefinedif the method doesn't exist, and calling.then()onundefinedthrows aTypeError.Suggested fix
- const toggleFullscreen = useCallback(() => { + const toggleFullscreen = useCallback(async () => { const el = containerRef.current if (!el) return - if (!document.fullscreenElement) { - el.requestFullscreen?.().then(() => setIsFullscreen(true)) - } else { - document.exitFullscreen?.().then(() => setIsFullscreen(false)) + try { + if (!document.fullscreenElement) { + if (el.requestFullscreen) { + await el.requestFullscreen() + } + } else if (document.exitFullscreen) { + await document.exitFullscreen() + } + } catch { + // Ignore rejected / unsupported fullscreen requests. } }, [])🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/components/OryArchitectureDiagram.tsx` around lines 191 - 198, The toggleFullscreen function uses optional chaining to call el.requestFullscreen?.() and document.exitFullscreen?.() but immediately chains .then(), which will throw if the call returns undefined or rejects; update toggleFullscreen to first check that the respective methods exist (el.requestFullscreen and document.exitFullscreen), call them only if present, capture their return value into a variable (promise), and then safely handle it by using Promise.resolve(promise).then(() => setIsFullscreen(true/false)).catch(() => {/* handle or log error and avoid changing state on failure */}); ensure you reference and update the toggleFullscreen function, containerRef, requestFullscreen, exitFullscreen, and setIsFullscreen usages.
🟡 Minor comments (9)
src/components/welcomePage/GuideLinkBox.tsx-12-22 (1)
12-22:⚠️ Potential issue | 🟡 MinorMake the whole card the link.
Lines 12-22 style this as a CTA box, but only the inline text/icon is clickable. The padded body becomes dead space, which is easy to miss on the welcome page. Wrapping the card itself in
Linkwill match the visual affordance.💡 Proposed fix
export function GuideLinkBox({ to, label }: GuideLinkBoxProps) { return ( - <div className="mt-ory-4 flex flex-col gap-ory-3 p-ory-6 rounded-ory border border-ory-border-info-tertiary bg-ory-info-tertiary"> - <Link - to={to} - className="ory-body-sm font-medium text-ory-info-on-tertiary no-underline inline-flex items-center gap-ory-2" - > + <Link + to={to} + className="mt-ory-4 flex flex-col gap-ory-3 rounded-ory border border-ory-border-info-tertiary bg-ory-info-tertiary p-ory-6 no-underline" + > + <span className="ory-body-sm inline-flex items-center gap-ory-2 font-medium text-ory-info-on-tertiary"> <IconBulb - className="w-5 h-5 shrink-0 text-ory-info-primary" + className="h-5 w-5 shrink-0 text-ory-info-primary" aria-hidden /> Follow the quickstart guide for {label} - </Link> - </div> + </span> + </Link> ) }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/components/welcomePage/GuideLinkBox.tsx` around lines 12 - 22, The CTA card currently only makes the inline text/icon clickable; wrap the entire card so the whole padded area is a link by replacing the outer <div className="..."> with the <Link to={to} className="..."> (or alternatively wrap the div with a single Link) and remove the inner Link to avoid nested anchors; keep the same class names, IconBulb, and the "Follow the quickstart guide for {label}" text so the full card (GuideLinkBox) is clickable and styling/ARIA are preserved.src/components/OryHeroDemo.jsx-132-135 (1)
132-135:⚠️ Potential issue | 🟡 MinorExpose the terminal stream as a live log.
The demo appends content in place, but this container is just a generic
div. Screen-reader users won't be told that output changed or that the final CTA appeared. Mark it as a log/live region and surface running state.♿ Suggested fix
<div ref={terminalRef} + role="log" + aria-live="polite" + aria-relevant="additions text" + aria-busy={isRunning} className="p-4 font-mono text-[13px] text-ory-text-primary bg-ory-bg-primary leading-[1.6] overflow-y-auto flex-1 min-h-0" >Also applies to: 189-199, 204-209
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/components/OryHeroDemo.jsx` around lines 132 - 135, The terminal output container (the div referenced by terminalRef) needs ARIA live-region semantics so screen-readers announce updates; add role="log" and aria-live="polite" (or "assertive" if announcements must interrupt) plus aria-atomic="true" or "false" as appropriate, and bind aria-busy to the component's running state (e.g., isStreaming / isRunning) so updates reflect live activity; apply the same changes to the other output containers mentioned (the divs around lines 189-199 and 204-209) and ensure the running state variable used in aria-busy is updated when the stream starts/stops.src/components/welcomePage/OverviewCard.tsx-49-57 (1)
49-57:⚠️ Potential issue | 🟡 MinorGive each inline CTA a unique accessible name.
Every welcome-page card currently renders the same
"Learn more"link text here, so assistive tech gets a list of indistinguishable links with no card context. Includetitlein the link’s accessible name, or make the heading part of the link.♿ Suggested fix
- <Link to={to} className="ory-guide-card__link mt-auto"> + <Link + to={to} + aria-label={`${linkLabel} about ${title}`} + className="ory-guide-card__link mt-auto" + > {linkLabel} <span aria-hidden>→</span> </Link>🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/components/welcomePage/OverviewCard.tsx` around lines 49 - 57, The link text "Learn more" is ambiguous to screen reader users; update the OverviewCard component so each CTA has a unique accessible name by adding a descriptive title attribute to the Link (e.g., title={`${title} - ${linkLabel}`}) or alternatively by making the card heading part of the link so the heading (title) and linkLabel are read together; modify the Link element (the JSX that uses variables linkLabel, to, wrapperClass, innerClass, content, and title) to include the chosen accessible-name change and ensure the aria-hidden decorative arrow remains aria-hidden.src/components/welcomePage/SelectFrameworkSection.tsx-17-20 (1)
17-20:⚠️ Potential issue | 🟡 MinorFix TypeScript capitalization in user-facing copy.
Line 17 and Line 20 use
Typescript; standard casing isTypeScript. This affects polish and search consistency.💡 Proposed fix
- label: "Typescript", + label: "TypeScript", ... - guideTitle: "Follow the quickstart guide for Typescript", + guideTitle: "Follow the quickstart guide for TypeScript",🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/components/welcomePage/SelectFrameworkSection.tsx` around lines 17 - 20, The displayed user-facing strings use "Typescript" with incorrect capitalization; update the values for the "label" and "guideTitle" properties in SelectFrameworkSection (the entries named label and guideTitle) to "TypeScript" (capital S) so both the option label and the quickstart guide text use the correct casing and remain consistent for search and polish.src/components/QuickStarts/constants.ts-109-113 (1)
109-113:⚠️ Potential issue | 🟡 MinorFix the Polis blurb typo.
“SCIM provising” should be “SCIM provisioning”.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/components/QuickStarts/constants.ts` around lines 109 - 113, The QuickStarts entry for label "Introduction to Ory Polis" contains a typo in the description property; change the string value of description from "Enterprise SSO for SAML and OIDC identity providers and SCIM provising." to "Enterprise SSO for SAML and OIDC identity providers and SCIM provisioning." so the description property in the QuickStarts constants (the object with label "Introduction to Ory Polis") uses the correct spelling.src/components/QuickStarts/CategoryFilter.tsx-17-31 (1)
17-31:⚠️ Potential issue | 🟡 MinorExpose the active category state.
The selected chip is only communicated by styling right now. Add
aria-pressedto the buttons so assistive tech can announce which category is currently applied.Suggested change
- <div className="flex flex-wrap items-center gap-2 mb-5"> + <div + className="flex flex-wrap items-center gap-2 mb-5" + role="group" + aria-label="Quickstart categories" + > @@ <button key={cat.id} type="button" + aria-pressed={activeCategory.id === cat.id} className={clsx(🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/components/QuickStarts/CategoryFilter.tsx` around lines 17 - 31, The category buttons only communicate selection visually; add an accessible state by setting the button's aria-pressed attribute inside the categories.map render so assistive tech can announce the active chip. Update the button in CategoryFilter.tsx (the mapped button using key={cat.id}, activeCategory and onCategoryChange) to include aria-pressed={activeCategory.id === cat.id} (or the equivalent boolean expression) so the selected category is exposed to assistive technologies.src/components/welcomePage/SolutionDesignStepper.tsx-159-170 (1)
159-170:⚠️ Potential issue | 🟡 MinorUse radio semantics for the answer choices.
The current selection is only visible by border color. Because this is a single-choice question, expose it as a radio group (
role="radiogroup"+role="radio"/aria-checked) or switch to actual radio inputs so screen readers can announce the selected answer.Suggested change
- <div className="flex flex-col gap-ory-3"> + <div + className="flex flex-col gap-ory-3" + role="radiogroup" + aria-label={currentQuestion.question} + > {currentQuestion.options.map((option) => { const selected = currentAnswer === option.value return ( <button key={option.value} type="button" + role="radio" + aria-checked={selected} onClick={() => handleAnswer(currentQuestion.id, option.value) }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/components/welcomePage/SolutionDesignStepper.tsx` around lines 159 - 170, The option buttons in SolutionDesignStepper render single-choice answers but lack radio semantics; update the markup around currentQuestion.options (in the SolutionDesignStepper component) to expose a proper radio group by either replacing the buttons with actual <input type="radio"> elements (wired to handleAnswer and currentAnswer) or by adding role="radiogroup" to the container and role="radio", aria-checked, and keyboard focus/keydown handling (Enter/Space) to each option element; ensure keys remain option.value and that selection styling still reflects currentAnswer so screen readers can announce the selected option.src/components/QuickStarts/LanguageFilter.tsx-37-59 (1)
37-59:⚠️ Potential issue | 🟡 MinorExpose the popup state on the trigger.
The toggle never tells assistive tech whether the language menu is open. Add
aria-expanded,aria-haspopup, and anaria-controlsrelationship to the popup container so this reads as a real disclosure instead of a generic button.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/components/QuickStarts/LanguageFilter.tsx` around lines 37 - 59, The button toggle in LanguageFilter.tsx should expose the popup state to assistive tech: update the toggle button (the element using onClick={() => setMenuOpen(...)} and the activeLanguage display) to include aria-expanded={menuOpen}, aria-haspopup="true", and aria-controls pointing to the popup container's id; add a stable id to the popup div (the element rendered when menuOpen is true) and ensure the popup container has that id and an appropriate role (e.g., role="menu") so the aria-controls relationship is valid.src/css/theme.css-1238-1245 (1)
1238-1245:⚠️ Potential issue | 🟡 MinorThis selector leaks the quickstart reset outside the tabs.
.tabs-container[data-group-id="language-quickstarts"] ~ * [class*="theme-code-block"]matches any descendant code block in any later sibling, so unrelated snippets further down the page inherit the quickstart styling too. Scope the reset to the tab container/panel instead.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/css/theme.css` around lines 1238 - 1245, The CSS selector `.tabs-container[data-group-id="language-quickstarts"] ~ * [class*="theme-code-block"]` leaks styles to later sibling content; restrict the reset to only code inside the quickstarts tab/panel by replacing the loose sibling descendant selector with one that targets the tab container/panel scope (e.g. target descendants of the `.tabs-container[data-group-id="language-quickstarts"]` panel element such as `.tabs-container[data-group-id="language-quickstarts"] .tab-panel [class*="theme-code-block"]` and likewise for the `pre`/`code` rules), so only code blocks inside the quickstarts tab inherit the quickstart styling.
| export default function SidebarDeploymentModel(): JSX.Element | null { | ||
| const history = useHistory() | ||
| const location = useLocation() | ||
| const sidebar = useDocsSidebar() | ||
| const quickstartsDeployment = useQuickstartsDeployment() | ||
| if (!sidebar?.name || !QUICKSTARTS_SIDEBAR_NAMES.has(sidebar.name)) | ||
| return null | ||
|
|
||
| const currentFromPath = getCurrentDeployment(location.pathname) | ||
| const current = quickstartsDeployment | ||
| ? DEPLOYMENT_OPTIONS.find( | ||
| (o) => o.id === quickstartsDeployment.deployment, | ||
| ) ?? currentFromPath | ||
| : currentFromPath | ||
| const [open, setOpen] = useState(false) | ||
| const ref = useRef<HTMLDivElement>(null) | ||
| const triggerImageUrl = DEPLOYMENT_IMAGES[current.id] | ||
|
|
||
| const handleSelect = (opt: (typeof DEPLOYMENT_OPTIONS)[number]) => { | ||
| if (quickstartsDeployment) { | ||
| quickstartsDeployment.setDeployment(opt.id as QuickstartsDeploymentId) | ||
| } | ||
| setOpen(false) | ||
| history.push("/docs/getting-started/overview") | ||
| } | ||
|
|
||
| useEffect(() => { | ||
| const handleClickOutside = (e: MouseEvent | TouchEvent | FocusEvent) => { | ||
| if (!ref.current?.contains(e.target as Node)) setOpen(false) | ||
| } | ||
| document.addEventListener("mousedown", handleClickOutside) | ||
| document.addEventListener("touchstart", handleClickOutside) | ||
| document.addEventListener("focusin", handleClickOutside) | ||
| return () => { | ||
| document.removeEventListener("mousedown", handleClickOutside) | ||
| document.removeEventListener("touchstart", handleClickOutside) | ||
| document.removeEventListener("focusin", handleClickOutside) | ||
| } | ||
| }, []) |
There was a problem hiding this comment.
Critical: Hooks called after conditional early return violates Rules of Hooks.
The useState, useRef, and useEffect hooks are called after the conditional return null on lines 59-60. React requires hooks to be called unconditionally and in the same order on every render. This will cause runtime errors when the sidebar condition changes.
🐛 Proposed fix - move hooks before conditional return
export default function SidebarDeploymentModel(): JSX.Element | null {
const history = useHistory()
const location = useLocation()
const sidebar = useDocsSidebar()
const quickstartsDeployment = useQuickstartsDeployment()
+ const [open, setOpen] = useState(false)
+ const ref = useRef<HTMLDivElement>(null)
+
+ useEffect(() => {
+ const handleClickOutside = (e: MouseEvent | TouchEvent | FocusEvent) => {
+ if (!ref.current?.contains(e.target as Node)) setOpen(false)
+ }
+ document.addEventListener("mousedown", handleClickOutside)
+ document.addEventListener("touchstart", handleClickOutside)
+ document.addEventListener("focusin", handleClickOutside)
+ return () => {
+ document.removeEventListener("mousedown", handleClickOutside)
+ document.removeEventListener("touchstart", handleClickOutside)
+ document.removeEventListener("focusin", handleClickOutside)
+ }
+ }, [])
+
if (!sidebar?.name || !QUICKSTARTS_SIDEBAR_NAMES.has(sidebar.name))
return null
const currentFromPath = getCurrentDeployment(location.pathname)
const current = quickstartsDeployment
? DEPLOYMENT_OPTIONS.find(
(o) => o.id === quickstartsDeployment.deployment,
) ?? currentFromPath
: currentFromPath
- const [open, setOpen] = useState(false)
- const ref = useRef<HTMLDivElement>(null)
const triggerImageUrl = DEPLOYMENT_IMAGES[current.id]
-
- // ... move handleSelect here ...
-
- useEffect(() => {
- // ... effect code ...
- }, [])🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@src/components/SidebarDeploymentModel/index.tsx` around lines 54 - 92, The
hooks (useState, useRef, useEffect, useHistory, useLocation, useDocsSidebar,
useQuickstartsDeployment) are being called after an early return which violates
the Rules of Hooks; move all hook calls and any derived values that rely on them
(history, location, sidebar, quickstartsDeployment, currentFromPath, current,
open/setOpen, ref, triggerImageUrl) to the top of SidebarDeploymentModel so they
always run before the conditional `if (!sidebar?.name ||
!QUICKSTARTS_SIDEBAR_NAMES.has(sidebar.name)) return null`, then keep the
conditional return only for rendering (leaving handleSelect and the useEffect
logic intact but relocated to use the hooks declared earlier).
| docs: "/docs/getting-started/integrate-auth/nextjs-app-router-quickstart", | ||
| }, |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "Find where example.docs is rendered:"
rg -n -C3 'href=\{.*\.docs\}|to=\{.*\.docs\}' src || true
echo
echo "Check for Docusaurus base-url helpers around example.docs:"
rg -n -C3 'useBaseUrl\(.*\.docs|<Link.*to=\{.*\.docs\}' src || true
echo
echo "Consumers of examples-content.tsx:"
rg -n -C2 'examples-content' src || trueRepository: ory/docs
Length of output: 1045
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "=== OverviewCard component definition ==="
find src -name "OverviewCard*" -o -name "*OverviewCard*" | head -20
echo
echo "=== Search for OverviewCard component ==="
rg -n "OverviewCard" src/components --type ts --type tsx -A 20 | head -60
echo
echo "=== Docusaurus baseUrl configuration ==="
rg -n "baseUrl" . --type ts --type tsx --type js -g "docusaurus*" | head -20
echo
echo "=== Check QuickStarts consumer usage of examples-content ==="
rg -n "examples-content\|example\.docs\|card\.docs" src/components/QuickStarts -A 5 -B 2 | head -80Repository: ory/docs
Length of output: 225
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "=== OverviewCard component implementation ==="
cat -n src/components/welcomePage/OverviewCard.tsx
echo
echo "=== Docusaurus config baseUrl ==="
rg -n "baseUrl" . -g "docusaurus*" 2>/dev/null | head -10
echo
echo "=== How QuickStarts hook uses examples ==="
cat -n src/components/QuickStarts/hooks/useExampleFilter.ts | head -50
echo
echo "=== Welcome component usage of OverviewCard ==="
cat -n src/components/Welcome/welcome.tsx | head -40Repository: ory/docs
Length of output: 5898
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "=== QuickStarts component that renders examples ==="
find src/components/QuickStarts -name "*.tsx" -o -name "*.ts" | xargs ls -la
echo
echo "=== Search for where examples.docs is rendered in QuickStarts ==="
rg -n "example\.docs\|ex\.docs" src/components/QuickStarts -A 3 -B 1
echo
echo "=== Check if there's a Link component in QuickStarts ==="
rg -n "Link.*to=\|href=" src/components/QuickStarts | head -20Repository: ory/docs
Length of output: 1245
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "=== QuickstartGrid.tsx (likely renders examples) ==="
cat -n src/components/QuickStarts/QuickstartGrid.tsx
echo
echo "=== quickstart-filter.tsx (main component) ==="
cat -n src/components/QuickStarts/quickstart-filter.tsx | head -100
echo
echo "=== Search for all Link imports in QuickStarts ==="
rg -n "from.*Link\|import.*Link" src/components/QuickStarts
echo
echo "=== Search broadly for docs field rendering ==="
rg -n "\.docs" src/components/QuickStarts -A 2 -B 1Repository: ory/docs
Length of output: 6283
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "=== ExampleList component (uses examples-content) ==="
find src/components/Examples -name "*.tsx" -o -name "*.ts" | xargs ls -la
echo
echo "=== example-list.tsx content ==="
cat -n src/components/Examples/example-list.tsx 2>/dev/null || find src -name "*example-list*" -type f
echo
echo "=== Search for how example.docs is used ==="
rg -n "example\.docs\|\.docs.*Link\|href.*docs" src -A 2 -B 1 | head -60Repository: ory/docs
Length of output: 8565
Fix baseUrl duplication in example doc links: remove /docs prefix from stored paths.
Both OverviewCard (Welcome page) and ExampleCard (Examples section) render the docs field through Docusaurus <Link> component, which automatically applies the baseUrl ("/docs/" in docusaurus.config.ts) to paths. Storing paths as /docs/getting-started/... causes the baseUrl to be applied twice, resulting in /docs/docs/getting-started/... links.
Change paths from /docs/... format to /... format so baseUrl resolution works correctly. For example: /docs/getting-started/integrate-auth/nextjs-app-router-quickstart → /getting-started/integrate-auth/nextjs-app-router-quickstart
Also applies to: 25-26, 34-35, 43-44, 52-53, 61-62, 70-71, 79-80, 106-107
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@src/pages/_assets/examples-content.tsx` around lines 16 - 17, The stored
example doc paths include a redundant "/docs" prefix which causes Docusaurus
Link to produce "/docs/docs/..." links; update the `docs` string values in
src/pages/_assets/examples-content.tsx (the objects used by OverviewCard and
ExampleCard) to remove the leading "/docs" so they start with "/" (e.g.,
"/getting-started/..." instead of "/docs/getting-started/..."); ensure every
`docs:` entry referenced by the OverviewCard and ExampleCard examples (including
those around the noted ranges) is updated consistently.
| interface MdxJsxAttribute { | ||
| name: string; | ||
| value: string | string[]; | ||
| } |
There was a problem hiding this comment.
🧩 Analysis chain
🌐 Web query:
In remark-mdx / mdast-util-mdx-jsx, what is the runtime and TypeScript shape of MdxJsxAttribute.valuefor JSX attributes written likelines={['network']}orfeatures={['console']}? Is it ever a native string[], or is it an mdxJsxAttributeValueExpression object?
💡 Result:
In remark-mdx / mdast-util-mdx-jsx, the runtime and TypeScript shape of MdxJsxAttribute.value for JSX attributes like lines={['network']} or features={['console']} is an MdxJsxAttributeValueExpression object, not a native string[]. The TypeScript type for MdxJsxAttribute is: interface MdxJsxAttribute extends Literal { type: 'mdxJsxAttribute' name: string value?: MdxJsxAttributeValueExpression | string | null | undefined } and MdxJsxAttributeValueExpression is: interface MdxJsxAttributeValueExpression extends Literal { type: 'mdxJsxAttributeValueExpression' value: string // The raw JS source: "['network']" data?: {estree?: Program | null | undefined} & Literal['data'] } At runtime, value holds the raw JavaScript expression source code as a string (e.g., ['network']), parsed into an estree AST in data.estree if addResult: true is passed to the parser. It is never a native string[]—arrays remain as unevaluated JS expressions in the mdast.
Citations:
- 1: https://github.com/syntax-tree/mdast-util-mdx-jsx
- 2: https://registry.npmjs.org/mdast-util-mdx-jsx
- 3: https://github.com/syntax-tree/mdast-util-mdx-jsx/blob/main/index.d.ts
- 4: https://github.com/syntax-tree/mdast-util-mdx-jsx/blob/main/readme.md
🏁 Script executed:
find . -name "productLinePlugin.ts" -o -name "productLinePlugin.js"Repository: ory/docs
Length of output: 86
🏁 Script executed:
cat -n src/plugins/productLinePlugin.ts | head -100Repository: ory/docs
Length of output: 4348
Expression-valued attributes are not decoded correctly; blocks will be unexpectedly filtered out.
The local MdxJsxAttribute type definition masks the actual MDX AST shape. When JSX attributes like lines={['network']} or features={['console']} are parsed, they arrive as mdxJsxAttributeValueExpression objects with raw JavaScript source in the value field, not as native string[]. The current logic:
Array.isArray(linesAttr.value)returnsfalse(the value is an object, not an array)- Wraps the expression object in an array:
[expressionObject] - Compares the product line string against the expression object, which never matches
This causes conditional blocks to be unexpectedly filtered out. Either parse the mdxJsxAttributeValueExpression.data.estree AST to extract the actual array value, or constrain the supported syntax to string literals only (e.g., lines="network").
Affects lines 67–86.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@src/plugins/productLinePlugin.ts` around lines 9 - 12, The MdxJsxAttribute
type and current logic incorrectly assume attribute.value is a string or
string[]; update the handling in productLinePlugin.ts to detect
expression-valued attributes (value.type === 'mdxJsxAttributeValueExpression')
for symbols like MdxJsxAttribute, linesAttr and featuresAttr and extract the
actual value by reading value.data.estree (parse the ESTree to recover string or
array literals), falling back to supporting only string literal attributes if
parsing fails; then normalize to string[] before doing membership checks so
comparisons against product line names succeed.
| // Replace component with plain text | ||
| (node as any).type = 'text'; | ||
| (node as any).value = productName; | ||
| delete (mdxNode as any).name; | ||
| delete (mdxNode as any).attributes; | ||
| delete (mdxNode as any).children; |
There was a problem hiding this comment.
🧩 Analysis chain
🌐 Web query:
In mdast, is it valid to replace an mdxJsxFlowElementwith atextnode, or to convert it to aparagraphwhile keeping block children such as headings, lists, or othermdxJsxFlowElements? What is the recommended way for a remark plugin to unwrap or remove a flow node?
💡 Result:
No, it is not valid in mdast to directly replace an mdxJsxFlowElement (a flow/block node) with a text node, as text is phrasing content and cannot appear where flow content is expected (such as in root or other block parents). Similarly, converting it to a paragraph while keeping its original block children (e.g., headings, lists, other mdxJsxFlowElements) is invalid, because paragraph requires PhrasingContent children only, not FlowContent or BlockContent like headings or lists. The recommended way for a remark plugin to unwrap or remove a flow node like mdxJsxFlowElement is to use tree-walking utilities such as unist-util-visit to find the node, then either: - Remove it entirely with unist-util-remove (mutates the tree in place). - Replace it by splicing its valid children (which are Array<BlockContent | DefinitionContent>) directly into its parent's children array at the correct index (unwrapping), ensuring the children match the parent's content model. - For selective unwrapping (e.g., single-paragraph children), replace the child paragraph with its contents, as shown in plugins like remark-unravel-mdx and remark-unwrap-images. Examples from unified docs and plugins confirm direct node replacement or removal is common, but must preserve content model validity (e.g., no block children in phrasing-only nodes).
Citations:
- 1: https://github.com/syntax-tree/mdast-util-mdx-jsx
- 2: https://github.com/syntax-tree/mdast/blob/main/readme.md
- 3: https://unifiedjs.com/explore/package/mdast-util-mdx-jsx/
- 4: https://registry.npmjs.org/remark-unravel-mdx
- 5: https://unifiedjs.com/explore/package/remark-unwrap-images/
- 6: https://egghead.io/lessons/javascript-remove-markdown-nodes-from-a-document-with-unist-util-remove
- 7: https://github.com/wooorm/mdast
- 8: https://github.com/etler/remark-unravel-mdx
- 9: https://unifiedjs.com/explore/package/unist-util-remove/
🏁 Script executed:
head -120 src/plugins/productLinePlugin.ts | tail -70Repository: ory/docs
Length of output: 2753
🏁 Script executed:
head -55 src/plugins/productLinePlugin.tsRepository: ory/docs
Length of output: 1798
Don't retag flow JSX nodes into text / paragraph.
These branches are rewriting mdxJsxFlowElement nodes into node kinds that are only valid in phrasing contexts. A hidden block becomes an invalid text node under a flow container, and a kept <ProductLineContent> turns into a paragraph even if its children contain headings, lists, or other flow nodes. Replace/remove the wrapper in parent.children using splice() instead of changing the node type in place.
Possible direction
-import { visit } from 'unist-util-visit';
+import { SKIP, visit } from 'unist-util-visit';
return (tree) => {
- visit(tree, (node: Node) => {
- const mdxNode = node as MdxJsxFlowElement;
+ visit(tree, 'mdxJsxFlowElement', (mdxNode: MdxJsxFlowElement, index, parent) => {
+ if (index == null || !parent) {
+ return;
+ }
// ...
if (mdxNode.type === 'mdxJsxFlowElement' && mdxNode.name === 'ProductLineContent') {
// ...
- if (shouldShow) {
- // Keep the children but remove the wrapper
- if (mdxNode.children) {
- (node as any).type = 'paragraph';
- delete (mdxNode as any).name;
- delete (mdxNode as any).attributes;
- }
- } else {
- // Remove this entire block from the output
- (node as any).type = 'text';
- (node as any).value = '';
- delete (mdxNode as any).name;
- delete (mdxNode as any).attributes;
- delete (mdxNode as any).children;
- }
+ parent.children.splice(index, 1, ...(shouldShow ? (mdxNode.children ?? []) : []));
+ return [SKIP, index];
}
});
};Also applies to: lines 89-103
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@src/plugins/productLinePlugin.ts` around lines 56 - 61, The code is
incorrectly retagging mdxJsxFlowElement nodes by mutating (node as
any).type/value and deleting mdxNode fields, which produces invalid phrasing
nodes; instead locate the wrapper in parent.children and replace or remove it
via parent.children.splice(index, 1, ...) with a new plain text node (or
multiple replacement nodes) so flow-level elements remain proper flow nodes; do
the same fix for the other branch handling lines 89-103 (where similar in-place
type mutation occurs), referencing the local variables node, mdxNode, and
parent.children and use the splice-based replacement to insert the desired
replacement nodes rather than modifying node.type in place.
Preview link: https://docs-rfbjzxqwi-ory.vercel.app/docs
Summary
This PR delivers a significant redesign of the Ory documentation site, introducing a new Homepage navigation experience, a new Quick Start page, restructured content architecture, and visual design improvements. It also establishes the foundational framework for separating documentation by deployment model: Ory Network, Ory OEL, and Ory OSS.
Changes
Homepage navigation
Replaced the previous homepage with a guided navigation experience that helps technical evaluators and developers orient quickly and find the right entry point for their use case. (Aligns better with our marketing site.)
Quick Start page
Added a new Quick Start page as a dedicated onboarding path for developers getting started with Ory for the first time.
Content restructuring
Reorganized existing documentation to support clearer content hierarchy. This restructuring is a prerequisite for the deployment-model separation work.
Product overview
Added new high-level product explanations with visual elements to demonstrate how each product fits within the context of an IAM system.
Visual design enhancements
Updated layout, typography, and component styling throughout the portal for improved readability and consistency. Introduced product color association.
Deployment model framework
Introduced the structural scaffolding to separate documentation by deployment context — Ory Network, Ory OEL, and Ory OSS — enabling product-specific content targeting in follow-on PRs.
What's not included
Full content separation by deployment model is out of scope for this PR. This change puts the framework in place; content migration and product-specific pages will follow.
Complete alignment on product names is out of the scope of this PR.
TESTING
New feature documentation (last few months)
Verify your new content appears in the redesigned navigation (docs/sidebar.ts). If it doesn't appear, add it to the /docs/sidebar.ts.
NOTE: There are several new sidebar files located within the /docs folder; if your content was added to the previous sidebar.ts file located in /src that file is no longer in use.
Content is complete
Verify that your product area contains the expected full set of content. The restructure may have affected navigation visibility for some pages.
Broken links
Spot-check links in your product area. Internal navigation files were significantly changed in this redesign.
Checklist
If this pull request addresses a security vulnerability,
I confirm that I got approval (please contact security@ory.com) from the maintainers to push the changes.
Summary by CodeRabbit
New Features
Documentation
Chores