-
Notifications
You must be signed in to change notification settings - Fork 42
fix: base docs domain #603
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
base: main
Are you sure you want to change the base?
Conversation
WalkthroughProgrammatic initialization replaces the previous inline data-configuration: a new initScalar flow builds dynamic config and targetServers, waits for Scalar availability, attempts multiple API entry points, falls back to data-attributes, and preserves subdomain state while improving DOM guards and UI behaviors. Changes
Sequence Diagram(s)sequenceDiagram
participant Page as Astro page
participant Init as initScalar
participant Scalar as Scalar bundle (external)
Note right of Page `#D3E4CD`: Component embeds container\nwith data-spec-url / data-proxy-url
Page->>Init: initScalar(container, options)
Init->>Init: waitForScalar / waitForElement / DOM ready
Init->>Scalar: try createApiReference(config)
alt createApiReference exists
Scalar-->>Init: ApiReference instance
else
Init->>Scalar: try ApiReference constructor
alt ApiReference exists
Scalar-->>Init: ApiReference instance
else
Init->>Scalar: try Scalar.default.createApiReference
alt fallback available
Scalar-->>Init: ApiReference instance
else
Init->>Page: populate data-attributes & fallback UI
end
end
end
Init->>Init: persist subdomain -> sessionStorage
Init->>Page: update UI (focus/select, auth labels, links)
Note left of Init `#FCE8C3`: Mutations & guards applied\n(no direct DOM casts)
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes
Pre-merge checks and finishing touches❌ Failed checks (1 inconclusive)
✅ Passed checks (1 passed)
✨ Finishing touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
Deploying kinde-docs-preview with
|
| Latest commit: |
91019db
|
| Status: | ✅ Deploy successful! |
| Preview URL: | https://dd999941.kinde-docs-preview.pages.dev |
| Branch Preview URL: | https://fix-api-docs-domain.kinde-docs-preview.pages.dev |
There was a problem hiding this 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
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
src/components/ScalarApiReference.astro(1 hunks)
🧰 Additional context used
🧠 Learnings (1)
📓 Common learnings
Learnt from: marcosmartini
Repo: kinde-oss/documentation PR: 226
File: src/content/docs/developer-tools/sdks/backend/nextjs-sdk.mdx:0-0
Timestamp: 2024-10-22T06:12:51.559Z
Learning: The Kinde documentation's base URL is not `kinde.com`, so links to the documentation should use the correct base URL.
Learnt from: marcosmartini
Repo: kinde-oss/documentation PR: 226
File: src/content/docs/developer-tools/sdks/backend/nextjs-sdk.mdx:0-0
Timestamp: 2024-10-22T06:13:23.808Z
Learning: In the Kinde documentation, links may use `localhost` URLs that are fixed to relative paths, and the docs base URL is not `kinde.com`.
872a11e to
900ac94
Compare
There was a problem hiding this 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
🧹 Nitpick comments (1)
src/components/ScalarApiReference.astro (1)
67-67: Improve type safety for Scalar API access.Lines 67 and 76 use unsafe
as anytype casts to access Scalar's internal APIs. While this is sometimes necessary for third-party libraries without TypeScript definitions, consider adding type guards to verify the existence of properties before accessing them to prevent runtime errors.Apply this diff to add type guards:
// Method 1: Try to access Scalar's internal API const scalarInstance = (window as any).ScalarApiReference; - if (scalarInstance && scalarInstance.updateConfiguration) { + if (scalarInstance && typeof scalarInstance.updateConfiguration === 'function') { scalarInstance.updateConfiguration({ servers: targetServers }); updated = true; } // Method 2: Access the parsed spec directly const scalarRef = (apiReferenceElement as any).__scalarApiReference; if (scalarRef && !updated) { - if (scalarRef.spec) { + if (scalarRef.spec && typeof scalarRef.spec === 'object') { scalarRef.spec.servers = targetServers; - if (scalarRef.update) { + if (typeof scalarRef.update === 'function') { scalarRef.update(); } updated = true; } // Also try to update configuration - if (scalarRef.configuration) { + if (scalarRef.configuration && typeof scalarRef.configuration === 'object') { scalarRef.configuration.servers = targetServers; updated = true; } }Also applies to: 76-76
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
src/components/ScalarApiReference.astro(2 hunks)
🧰 Additional context used
🧠 Learnings (1)
📓 Common learnings
Learnt from: marcosmartini
Repo: kinde-oss/documentation PR: 226
File: src/content/docs/developer-tools/sdks/backend/nextjs-sdk.mdx:0-0
Timestamp: 2024-10-22T06:12:51.559Z
Learning: The Kinde documentation's base URL is not `kinde.com`, so links to the documentation should use the correct base URL.
Learnt from: marcosmartini
Repo: kinde-oss/documentation PR: 226
File: src/content/docs/developer-tools/sdks/backend/nextjs-sdk.mdx:0-0
Timestamp: 2024-10-22T06:13:23.808Z
Learning: In the Kinde documentation, links may use `localhost` URLs that are fixed to relative paths, and the docs base URL is not `kinde.com`.
🔇 Additional comments (3)
src/components/ScalarApiReference.astro (3)
16-21: LGTM: Variable naming is now consistent.The variable name (
your_kinde_subdomainwith underscores) on line 18, its default value on line 19, and the validation check on line 206 are all consistent. This resolves the inconsistency flagged in the previous review.Also applies to: 206-206
16-16: LGTM: HTTPS and JSON serialization properly implemented.The server URL correctly uses HTTPS (line 16), addressing the security concern from the previous review. The configuration is properly serialized using
JSON.stringify()(line 26), ensuring valid JSON with quoted keys in thedata-configurationattribute (line 33).Also applies to: 26-26, 33-33
93-104: Method 3 has a logical issue with template URL matching.The code attempts to set an HTMLSelectElement's value to the template URL
"https://{your_kinde_subdomain}.kinde.com". However, HTML select elements can only be set to values that exist as actual<option>children. If Scalar's server dropdown doesn't expose the template URL as an option value—or structures the dropdown differently—this assignment will fail silently without error.Methods 1 and 2 (lines 67-90) properly update Scalar's internal configuration/spec objects. Method 3 relies on fragile DOM assumptions as a fallback. Consider:
- Verifying the actual structure of Scalar's server selector by inspecting it in the browser
- Validating that the template URL actually exists as a select option before attempting assignment
- Adding fallback error handling or logging to confirm whether Method 3 succeeds
900ac94 to
c481b06
Compare
There was a problem hiding this 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 (2)
src/components/ScalarApiReference.astro (2)
149-157: Add null check for parentElement.Line 150 accesses
subdomainElement.parentElementwithout checking if it exists. If the element is detached from the DOM or doesn't have a parent, this will cause a null pointer exception on line 153.Apply this diff:
function checkSubdomain(subdomainElement) { const parentEl = subdomainElement.parentElement; + if (!parentEl) return; if (subdomainElement.value === "your_kinde_subdomain" || subdomainElement.value.trim() === "") { parentEl.style.animation = `pulse 1s linear infinite`; } else { parentEl.style.animation = `none`; } }
201-222: Add cleanup for MutationObserver to prevent memory leaks.The MutationObserver is created and starts observing on line 222, but it's never disconnected. This can cause memory leaks if the page navigates away or when using Astro's view transitions feature, as the observer will continue running even after the component is no longer needed.
Add cleanup logic:
// Start observing for when the .api-client-drawer is added/removed from the DOM tree observer.observe(document.body, {childList: true, subtree: true}); + + // Cleanup observer when navigating away + document.addEventListener('astro:before-preparation', () => { + observer.disconnect(); + }, { once: true });Note: If you're not using Astro's view transitions, you can use
beforeunloadinstead:window.addEventListener('beforeunload', () => { observer.disconnect(); }, { once: true });
🧹 Nitpick comments (1)
src/components/ScalarApiReference.astro (1)
86-97: Minor cleanup: Remove redundant code.Two small issues:
- Line 92: The container already has
id="api-reference"from line 10, so reassigning it is unnecessary.- Line 97: The
window.__scalarApiReferenceproperty is stored but never used elsewhere in the code.Apply this diff:
container.setAttribute('data-configuration', JSON.stringify({ theme: "deepSpace", hideDarkModeToggle: true, searchHotKey: "j", servers: targetServers })); - container.id = 'api-reference'; return; } - // Store reference for potential updates later - window.__scalarApiReference = apiReference; }
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
src/components/ScalarApiReference.astro(7 hunks)
🧰 Additional context used
🧠 Learnings (2)
📓 Common learnings
Learnt from: marcosmartini
Repo: kinde-oss/documentation PR: 226
File: src/content/docs/developer-tools/sdks/backend/nextjs-sdk.mdx:0-0
Timestamp: 2024-10-22T06:12:51.559Z
Learning: The Kinde documentation's base URL is not `kinde.com`, so links to the documentation should use the correct base URL.
Learnt from: marcosmartini
Repo: kinde-oss/documentation PR: 226
File: src/content/docs/developer-tools/sdks/backend/nextjs-sdk.mdx:0-0
Timestamp: 2024-10-22T06:13:23.808Z
Learning: In the Kinde documentation, links may use `localhost` URLs that are fixed to relative paths, and the docs base URL is not `kinde.com`.
📚 Learning: 2024-11-12T06:00:08.396Z
Learnt from: marcosmartini
Repo: kinde-oss/documentation PR: 253
File: src/content/docs/properties/work-with-properties/property-groups.mdx:13-13
Timestamp: 2024-11-12T06:00:08.396Z
Learning: In the project documentation, the `/kinde-apis/management` and `/kinde-apis/frontend` pages use a third-party API reference and client that loads a Single Page Application (SPA). Therefore, links to sections within these pages are valid, even if they appear to point to non-existent sections in static analysis, and should not be flagged.
Applied to files:
src/components/ScalarApiReference.astro
🔇 Additional comments (2)
src/components/ScalarApiReference.astro (2)
20-29: LGTM!The server configuration correctly uses HTTPS, and the variable naming with underscores is consistent with the validation logic on line 152.
203-217: LGTM!The defensive checks for
node.nodeType,node.classList, anddocument.bodyare excellent safeguards against potential runtime errors when manipulating the DOM.
| const config = { | ||
| target: container, | ||
| url: specURL, | ||
| configuration: { | ||
| theme: "deepSpace", | ||
| hideDarkModeToggle: true, | ||
| searchHotKey: "j", | ||
| servers: targetServers | ||
| } | ||
| }; | ||
|
|
||
| // Add proxy if provided | ||
| if (proxyURL) { | ||
| config.proxy = proxyURL; | ||
| } |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
🧩 Analysis chain
Verify the Scalar configuration object structure.
The current structure nests theme, hideDarkModeToggle, searchHotKey, and servers under a configuration key. However, based on common Scalar API Reference patterns, these properties are typically placed at the top level of the config object, alongside spec or url.
If this structure is incorrect, Scalar initialization may fail silently and fall back to the data-attributes approach, which could result in unexpected behavior.
Please verify the correct structure by checking the Scalar API Reference documentation:
If the configuration properties should be at the top level, apply this diff:
- const config = {
- target: container,
- url: specURL,
- configuration: {
- theme: "deepSpace",
- hideDarkModeToggle: true,
- searchHotKey: "j",
- servers: targetServers
- }
- };
+ const config = {
+ target: container,
+ spec: {
+ url: specURL
+ },
+ theme: "deepSpace",
+ hideDarkModeToggle: true,
+ searchHotKey: "j",
+ servers: targetServers
+ };🌐 Web query:
Scalar API Reference createApiReference configuration object structure
💡 Result:
Top-level configuration for Scalar.createApiReference is a universal object (or array of such objects). Key structure and common properties:
- Root (object or array of objects). If array: multiple document/configurations; first is default unless one has default: true. [1]
- OpenAPI source:
- url: string (absolute/relative URL to JSON/YAML). [1]
- content: string (inline OpenAPI JSON/YAML). [1]
- sources: array of source objects (each with url/content/title/slug/default). [1]
- Document metadata (per config/source):
- title, slug, default (boolean). [1]
- Appearance / behavior:
- customCss: string (CSS to inject). [1]
- proxyUrl (used in examples for cross-origin/proxying). [2]
- Authentication (authentication object / AuthenticationConfiguration) — can prefill security schemes and preferredSecurityScheme, and has entries for apiKey/httpBearer/httpBasic/oauth2 flows. [1]
- JS API helpers:
- createApiReference(selectorOrElement, config) returns an app with methods like mount(), updateConfiguration(newConfig), destroy(). [2]
For full, exhaustive list and examples see the Scalar configuration docs. [1][2]
Fix the Scalar configuration object structure—all options should be at the top level.
Scalar configuration accepts properties like hideDarkModeToggle, searchHotKey, servers, theme, and proxyUrl as top-level configuration object properties. The current code incorrectly nests these under a configuration key, which doesn't exist in the Scalar API.
Pass configuration directly to Scalar.createApiReference, with url as a top-level property. Apply this fix:
- const config = {
- target: container,
- url: specURL,
- configuration: {
- theme: "deepSpace",
- hideDarkModeToggle: true,
- searchHotKey: "j",
- servers: targetServers
- }
- };
+ const config = {
+ target: container,
+ url: specURL,
+ theme: "deepSpace",
+ hideDarkModeToggle: true,
+ searchHotKey: "j",
+ servers: targetServers
+ };🤖 Prompt for AI Agents
In src/components/ScalarApiReference.astro around lines 56 to 70, the Scalar
config incorrectly nests options under a configuration key and uses proxy
instead of proxyUrl; Scalar expects url and options like theme,
hideDarkModeToggle, searchHotKey, servers, and proxyUrl at the top level. Move
theme, hideDarkModeToggle, searchHotKey, and servers out of the nested
configuration object into the top-level config alongside target and url, and set
proxyUrl (not proxy) when proxyURL is provided, then pass that top-level config
into Scalar.createApiReference.
| waitForElement(".introduction-card", (parentEl) => { | ||
| const subdomainInputElement = parentEl.querySelector("input#variable-subdomain"); | ||
| const tokenInputElement = parentEl.querySelector(".authentication-content input"); | ||
| const urlParams = new URLSearchParams(window.location.search); | ||
|
|
||
|
|
||
| // Check for "token" in URL params | ||
| if (urlParams.has('token')) { | ||
| const token = urlParams.get('token'); | ||
| tokenInputElement.value = token as string; | ||
| tokenInputElement.value = token; | ||
| tokenInputElement.dispatchEvent(new Event("input")); | ||
| urlParams.delete('token'); | ||
| } |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Add null check for tokenInputElement.
Line 161 uses querySelector which can return null if the authentication input doesn't exist. Lines 168-169 then access properties on tokenInputElement without checking, which will cause a null pointer exception if the element isn't found.
Apply this diff:
waitForElement(".introduction-card", (parentEl) => {
const subdomainInputElement = parentEl.querySelector("input#variable-subdomain");
const tokenInputElement = parentEl.querySelector(".authentication-content input");
const urlParams = new URLSearchParams(window.location.search);
// Check for "token" in URL params
- if (urlParams.has('token')) {
+ if (urlParams.has('token') && tokenInputElement) {
const token = urlParams.get('token');
tokenInputElement.value = token;
tokenInputElement.dispatchEvent(new Event("input"));
urlParams.delete('token');
}📝 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.
| waitForElement(".introduction-card", (parentEl) => { | |
| const subdomainInputElement = parentEl.querySelector("input#variable-subdomain"); | |
| const tokenInputElement = parentEl.querySelector(".authentication-content input"); | |
| const urlParams = new URLSearchParams(window.location.search); | |
| // Check for "token" in URL params | |
| if (urlParams.has('token')) { | |
| const token = urlParams.get('token'); | |
| tokenInputElement.value = token as string; | |
| tokenInputElement.value = token; | |
| tokenInputElement.dispatchEvent(new Event("input")); | |
| urlParams.delete('token'); | |
| } | |
| waitForElement(".introduction-card", (parentEl) => { | |
| const subdomainInputElement = parentEl.querySelector("input#variable-subdomain"); | |
| const tokenInputElement = parentEl.querySelector(".authentication-content input"); | |
| const urlParams = new URLSearchParams(window.location.search); | |
| // Check for "token" in URL params | |
| if (urlParams.has('token') && tokenInputElement) { | |
| const token = urlParams.get('token'); | |
| tokenInputElement.value = token; | |
| tokenInputElement.dispatchEvent(new Event("input")); | |
| urlParams.delete('token'); | |
| } |
🤖 Prompt for AI Agents
In src/components/ScalarApiReference.astro around lines 159 to 171,
tokenInputElement is queried with querySelector() and may be null; add a null
check before accessing its .value or dispatching events. Update the code to
verify tokenInputElement exists (e.g., if (tokenInputElement) { ... }) and only
then assign tokenInputElement.value, dispatch the input event, and delete the
token param; keep behavior unchanged when the element is present and do nothing
when it's absent.
c481b06 to
2e389de
Compare
2e389de to
91019db
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 0
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
src/components/ScalarApiReference.astro (1)
197-236: Add null checks for input elements to prevent runtime errors.Lines 198-199 use
querySelector, which can returnnullif elements don't exist. Line 206 accessestokenInputElement.valuewithout checking for null, and the subdomain element is used throughout without verification. This will throw aTypeErrorif either element is missing.This issue was previously flagged but remains unfixed.
Apply this diff:
waitForElement(".introduction-card", (parentEl) => { const subdomainInputElement = parentEl.querySelector("input#variable-subdomain"); const tokenInputElement = parentEl.querySelector(".authentication-content input"); + + if (!subdomainInputElement) { + console.warn('Subdomain input element not found'); + return; + } + const urlParams = new URLSearchParams(window.location.search); // Check for "token" in URL params - if (urlParams.has('token')) { + if (urlParams.has('token') && tokenInputElement) { const token = urlParams.get('token'); tokenInputElement.value = token; tokenInputElement.dispatchEvent(new Event("input")); urlParams.delete('token'); }
♻️ Duplicate comments (1)
src/components/ScalarApiReference.astro (1)
89-103: Fix configuration structure—options must be at top level, not nested.The configuration object incorrectly nests
theme,hideDarkModeToggle,searchHotKey, andserversunder aconfigurationkey (line 92). Scalar's API expects these as top-level properties alongsidetargetandurl. Additionally, line 102 uses the wrong property name:proxyinstead ofproxyUrl.This was previously flagged and marked as addressed, but the code still contains both issues.
Apply this diff:
const config = { target: container, url: specURL, - configuration: { - theme: "deepSpace", - hideDarkModeToggle: true, - searchHotKey: "j", - servers: targetServers - } + theme: "deepSpace", + hideDarkModeToggle: true, + searchHotKey: "j", + servers: targetServers }; // Add proxy if provided if (proxyURL) { - config.proxy = proxyURL; + config.proxyUrl = proxyURL; }Based on library documentation.
🧹 Nitpick comments (3)
src/components/ScalarApiReference.astro (3)
16-25: Add timeout limit to prevent infinite polling.The
waitForElementfunction has no maximum retry limit, so it will poll indefinitely if the selector never matches. This can cause memory leaks and unnecessary CPU usage, especially if the element is removed or never added to the DOM.Apply this diff to add a timeout:
- function waitForElement(selector, callback) { + function waitForElement(selector, callback, maxRetries = 50) { const element = document.querySelector(selector); if (element) { callback(element); - } else { + } else if (maxRetries > 0) { - setTimeout(() => waitForElement(selector, callback), 100); + setTimeout(() => waitForElement(selector, callback, maxRetries - 1), 100); + } else { + console.warn(`Element ${selector} not found after polling`); } }
156-170: Consider using waitForElement for consistency.Line 165 uses
document.querySelectordirectly while the surrounding code uses thewaitForElementpattern. Although the null check on line 167 makes this safe, usingwaitForElementwould be more consistent and ensure the element exists before attempting to modify it.Consider this approach:
waitForElement('.authentication-content label', (element) => { if (window.location.pathname.includes('management')) { element.textContent = "M2M Token" } if (window.location.pathname.includes('frontend')) { element.textContent = "User access token" } - const authenticationLabel = document.querySelector('.security-scheme-label'); - - if (authenticationLabel) { - authenticationLabel.textContent = "Authentication"; - } + waitForElement('.security-scheme-label', (authenticationLabel) => { + authenticationLabel.textContent = "Authentication"; + }); })
238-260: Add cleanup to disconnect MutationObserver.The
MutationObserveris created and starts observing on line 260, but it's never disconnected. This can cause memory leaks if the component is destroyed or the page navigates away, as the observer continues running and holding references.Add cleanup logic:
// Start observing for when the .api-client-drawer is added/removed from the DOM tree - observer.observe(document.body, {childList: true, subtree: true}); + if (document.body) { + observer.observe(document.body, {childList: true, subtree: true}); + } + + // Cleanup on page navigation + window.addEventListener('beforeunload', () => { + observer.disconnect(); + });Alternatively, if Astro supports lifecycle hooks in this context, use those for proper cleanup.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
src/components/ScalarApiReference.astro(7 hunks)
🧰 Additional context used
🧠 Learnings (1)
📓 Common learnings
Learnt from: marcosmartini
Repo: kinde-oss/documentation PR: 226
File: src/content/docs/developer-tools/sdks/backend/nextjs-sdk.mdx:0-0
Timestamp: 2024-10-22T06:12:51.559Z
Learning: The Kinde documentation's base URL is not `kinde.com`, so links to the documentation should use the correct base URL.
Learnt from: marcosmartini
Repo: kinde-oss/documentation PR: 226
File: src/content/docs/developer-tools/sdks/backend/nextjs-sdk.mdx:0-0
Timestamp: 2024-10-22T06:13:23.808Z
Learning: In the Kinde documentation, links may use `localhost` URLs that are fixed to relative paths, and the docs base URL is not `kinde.com`.
🔇 Additional comments (3)
src/components/ScalarApiReference.astro (3)
33-42: LGTM!The
targetServersconfiguration correctly defines the Kinde subdomain as a server variable with proper OpenAPI structure.
51-86: LGTM! Robust fallback mechanism.The retry logic with timeout and multiple global reference checks is well-implemented. The data-attribute fallback ensures initialization succeeds even if Scalar loads differently than expected.
Note: The fallback configuration (lines 78-83) correctly uses flat JSON structure for data-attributes, which differs from the programmatic config on lines 89-98 that still has nesting issues.
138-143: LGTM!The DOM ready check correctly handles both loading and loaded states to ensure
initScalarruns at the appropriate time.
Explain your changes
Update the base url to be configurable
Checklist
🛟 If you need help, consider asking for advice over in the Kinde community.
Summary by CodeRabbit
Documentation
New Features
Bug Fixes / UX