Conversation
WalkthroughThis update introduces several enhancements across documentation and asset files. New global variables with encoded string data have been added in JavaScript files. A new JavaScript file is introduced for managing SVG icons, and a translations object enriches UI localization. CSS files have been updated with new color variables, classes, and layering directives. Additionally, multiple HTML documentation files for various enumerations have been either newly added or restructured with improved metadata, script management, and consistent identifier formatting. Changes
Suggested reviewers
Poem
✨ Finishing Touches
🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
Documentation and Community
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (1)
packages/xrpl/typedoc.js (1)
1-1: Remove redundant 'use strict' directive.JavaScript modules are automatically in strict mode, so this directive is unnecessary.
-'use strict'🧰 Tools
🪛 Biome (1.9.4)
[error] 1-1: Redundant use strict directive.
The entire contents of JavaScript modules are automatically in strict mode, with no statement needed to initiate it.
Safe fix: Remove the redundant use strict directive.(lint/suspicious/noRedundantUseStrict)
📜 Review details
Configuration used: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (5)
packages/xrpl/package.json(1 hunks)packages/xrpl/tsconfig.docs.json(0 hunks)packages/xrpl/typedoc.js(1 hunks)packages/xrpl/typedoc.json(1 hunks)packages/xrpl/typedoc.tsx(0 hunks)
💤 Files with no reviewable changes (2)
- packages/xrpl/tsconfig.docs.json
- packages/xrpl/typedoc.tsx
✅ Files skipped from review due to trivial changes (1)
- packages/xrpl/typedoc.json
🧰 Additional context used
🪛 Biome (1.9.4)
packages/xrpl/typedoc.js
[error] 1-1: Redundant use strict directive.
The entire contents of JavaScript modules are automatically in strict mode, with no statement needed to initiate it.
Safe fix: Remove the redundant use strict directive.
(lint/suspicious/noRedundantUseStrict)
⏰ Context from checks skipped due to timeout of 90000ms (11)
- GitHub Check: integration (22.x)
- GitHub Check: unit (20.x)
- GitHub Check: unit (18.x)
- GitHub Check: snippets (22.x)
- GitHub Check: integration (20.x)
- GitHub Check: snippets (20.x)
- GitHub Check: integration (18.x)
- GitHub Check: browser (18.x)
- GitHub Check: snippets (18.x)
- GitHub Check: Socket Security: Pull Request Alerts
- GitHub Check: Analyze (javascript)
🔇 Additional comments (7)
packages/xrpl/package.json (1)
61-61: Docgen script simplified to directly use typedoc.The change successfully removes the TypeScript compilation step, addressing the issue with ECMAScript module compatibility mentioned in the PR objectives. This change now directly runs Typedoc and then appends the domain to the CNAME file.
packages/xrpl/typedoc.js (6)
2-4: Module exports setup looks good.The CommonJS-style exports setup correctly defines this file as a module with an exported
loadfunction.
5-11: GTM script properly defined as a template literal.The Google Tag Manager initialization script is correctly defined as a template literal, making it easier to read.
12-14: Dynamic import approach resolves the ECMAScript module compatibility issue.Using dynamic import for the
typedocdependency correctly addresses the compatibility issue mentioned in the PR objectives. This approach eliminates the need for transpilation from TSX to JS.
15-28: Head hook correctly injects Osano and GTM scripts.The implementation uses JSX to create and inject the necessary scripts into the document head.
29-41: Body hook properly adds the GTM noscript fallback.The noscript element with the iframe for Google Tag Manager is correctly implemented as a fallback for users with JavaScript disabled.
42-42: Function properly exported.The
loadfunction is correctly exported, making it available for the documentation generation process.
|
I raised this issue with the dependency maintainer here: TypeStrong/typedoc#2870 |
packages/xrpl/typedoc.js
Outdated
| @@ -0,0 +1,42 @@ | |||
| 'use strict' | |||
There was a problem hiding this comment.
Can we leave a TODO here (or open an issue) to switch back to TS?
There was a problem hiding this comment.
@mvadari Is there a way to switch back to typescript without switching from commonjs -> ESM ?
Patel-Raj
force-pushed
the
docgen-issue-fix
branch
from
2ee3406 to
9d55cb3
Compare
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (5)
packages/xrpl/typedoc.mts (5)
1-1: Consider adding explicit type for the Application parameter.The import statement is correct, but it would be beneficial to also import and use the
Applicationtype from typedoc for stronger type checking.-import { JSX } from 'typedoc' +import { JSX, Application } from 'typedoc'
3-9: Extract the GTM ID as a constant for reusability.The GTM container ID is hardcoded in both the script and later in the iframe source. Extracting it as a constant would improve maintainability.
+const GTM_CONTAINER_ID = 'GTM-M7HKJJ3' + const GTMScript = ` (function(w,d,s,l,i){w[l]=w[l]||[];w[l].push({'gtm.start': new Date().getTime(),event:'gtm.js'});var f=d.getElementsByTagName(s)[0], j=d.createElement(s),dl=l!='dataLayer'?'&l='+l:'';j.async=true;j.src= 'https://www.googletagmanager.com/gtm.js?id='+i+dl;f.parentNode.insertBefore(j,f); -})(window,document,'script','dataLayer','GTM-M7HKJJ3'); +})(window,document,'script','dataLayer','${GTM_CONTAINER_ID}'); `
1-38: Add JSDoc comments to describe the purpose and usage of this module.This file lacks documentation explaining its purpose and how it integrates with the documentation generation process. Adding JSDoc comments would improve maintainability.
import { JSX } from 'typedoc' +/** + * Google Tag Manager container ID used for analytics + */ +const GTM_CONTAINER_ID = 'GTM-M7HKJJ3' + const GTMScript = ` (function(w,d,s,l,i){w[l]=w[l]||[];w[l].push({'gtm.start': new Date().getTime(),event:'gtm.js'});var f=d.getElementsByTagName(s)[0], j=d.createElement(s),dl=l!='dataLayer'?'&l='+l:'';j.async=true;j.src= 'https://www.googletagmanager.com/gtm.js?id='+i+dl;f.parentNode.insertBefore(j,f); -})(window,document,'script','dataLayer','GTM-M7HKJJ3'); +})(window,document,'script','dataLayer','${GTM_CONTAINER_ID}'); ` +/** + * TypeDoc plugin that integrates Google Tag Manager and Osano consent management + * into the generated documentation. + * + * @param app - The TypeDoc application instance + */ export function load(app) {
26-37: Use the extracted GTM container ID constant in the iframe source.For consistency, use the same GTM container ID constant in the iframe source as was used in the script.
app.renderer.hooks.on('body.begin', () => JSX.createElement( 'noscript', null, JSX.createElement('iframe', { - src: 'https://www.googletagmanager.com/ns.html?id=GTM-M7HKJJ3', + src: `https://www.googletagmanager.com/ns.html?id=${GTM_CONTAINER_ID}`, height: 0, width: 0, style: 'display:none;visibility:hidden', }), ), )
11-38: Add error handling to the hook registration.Consider adding error handling to handle cases where the hooks might fail. This would make the code more robust.
export function load(app) { + try { app.renderer.hooks.on('head.begin', () => JSX.createElement( JSX.Fragment, null, JSX.createElement('script', { src: 'https://cmp.osano.com/AzyjT6TIZMlgyLyy8/ad2447d5-f101-40df-b92e-d6452b5ecac0/osano.js', }), JSX.createElement( 'script', null, JSX.createElement(JSX.Raw, { html: GTMScript }), ), ), ) app.renderer.hooks.on('body.begin', () => JSX.createElement( 'noscript', null, JSX.createElement('iframe', { src: 'https://www.googletagmanager.com/ns.html?id=GTM-M7HKJJ3', height: 0, width: 0, style: 'display:none;visibility:hidden', }), ), ) + } catch (error) { + console.error('Failed to register TypeDoc hooks for GTM integration:', error) + } }
📜 Review details
Configuration used: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (4)
packages/xrpl/tsconfig.docs.json(1 hunks)packages/xrpl/typedoc.json(1 hunks)packages/xrpl/typedoc.mts(1 hunks)packages/xrpl/typedoc.tsx(0 hunks)
💤 Files with no reviewable changes (1)
- packages/xrpl/typedoc.tsx
🚧 Files skipped from review as they are similar to previous changes (2)
- packages/xrpl/tsconfig.docs.json
- packages/xrpl/typedoc.json
⏰ Context from checks skipped due to timeout of 90000ms (20)
- GitHub Check: unit (22.x)
- GitHub Check: unit (20.x)
- GitHub Check: snippets (22.x)
- GitHub Check: integration (22.x)
- GitHub Check: snippets (20.x)
- GitHub Check: unit (18.x)
- GitHub Check: snippets (18.x)
- GitHub Check: snippets (20.x)
- GitHub Check: integration (22.x)
- GitHub Check: integration (20.x)
- GitHub Check: integration (20.x)
- GitHub Check: snippets (18.x)
- GitHub Check: integration (18.x)
- GitHub Check: integration (18.x)
- GitHub Check: build-and-lint (18.x)
- GitHub Check: build-and-lint (18.x)
- GitHub Check: browser (18.x)
- GitHub Check: Analyze (javascript)
- GitHub Check: browser (18.x)
- GitHub Check: Analyze (javascript)
🔇 Additional comments (1)
packages/xrpl/typedoc.mts (1)
1-38:Details
❓ Verification inconclusive
Verify that this implementation resolves the docgen issue.
This new module-based approach appears to address the issue mentioned in the PR objectives, where there was a problem with transpilation of
typedoc.tsxtotypedoc.js. The use of.mtsfile extension and direct JSX usage from typedoc should resolve the incompatibility with ECMAScript modules.To confirm this implementation resolves the issue, you could run:
🏁 Script executed:
#!/bin/bash # Check if there are any references to the old typedoc.tsx file in the project echo "Checking for references to the old file..." rg --files-with-matches "typedoc\.tsx" || echo "No references found" # Check if the npm run docgen command works with this new implementation echo "Checking if docgen script references the new file..." rg --json "docgen" package.json | jq -r '.data.text'Length of output: 370
Review Update: Verify Docgen Implementation
- The search confirms there are no references to the old
typedoc.tsxfile in the repository.- The check for a "docgen" reference in
package.jsonreturned null. This might indicate that the new module-based approach isn’t explicitly referenced there. Please verify manually that running the docgen process actually uses the newtypedoc.mtsimplementation and generates documentation as expected.
packages/xrpl/typedoc.mts
Outdated
| })(window,document,'script','dataLayer','GTM-M7HKJJ3'); | ||
| ` | ||
|
|
||
| export function load(app) { |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Add a type annotation to the load function parameter.
The function is missing a type annotation for the app parameter. Since this is a TypeScript module, adding proper type annotations would improve type safety.
-export function load(app) {
+export function load(app: Application) {📝 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.
| export function load(app) { | |
| export function load(app: Application) { |
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (1)
packages/xrpl/typedoc.mts (1)
6-6: Consider externalizing the GTM container ID.While not a critical issue, consider moving the GTM container ID to a configuration file or environment variable to make it easier to change in different environments.
-const GTM_CONTAINER_ID = 'GTM-M7HKJJ3' +// Import from a configuration file or use an environment variable +const GTM_CONTAINER_ID = process.env.GTM_CONTAINER_ID || 'GTM-M7HKJJ3'
📜 Review details
Configuration used: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
packages/xrpl/typedoc.mts(1 hunks)
⏰ Context from checks skipped due to timeout of 90000ms (12)
- GitHub Check: unit (22.x)
- GitHub Check: snippets (22.x)
- GitHub Check: integration (22.x)
- GitHub Check: unit (20.x)
- GitHub Check: snippets (20.x)
- GitHub Check: unit (18.x)
- GitHub Check: integration (20.x)
- GitHub Check: browser (18.x)
- GitHub Check: snippets (18.x)
- GitHub Check: integration (18.x)
- GitHub Check: Analyze (javascript)
- GitHub Check: build-and-lint (18.x)
🔇 Additional comments (6)
packages/xrpl/typedoc.mts (6)
1-1: Import looks good.The import statement correctly includes the necessary TypeDoc dependencies for the module.
3-6: Documentation is clear and informative.The comment clearly explains what the constant represents.
8-14: Google Tag Manager script is properly structured.The GTM initialization script is correctly defined as a template string with the container ID variable properly interpolated.
16-22: Function is now properly typed.The function signature now includes the type annotation for the
appparameter, addressing the previous review comment.
23-36: Head hooks implementation looks correct.The implementation correctly adds both the Osano script and GTM initialization script to the document head using TypeDoc's renderer hooks.
37-49: Body hooks implementation looks correct.The noscript fallback for GTM is properly implemented to ensure tracking works even when JavaScript is disabled.
| "module": "commonjs", | ||
| "esModuleInterop": true, | ||
| "target": "ES2015" | ||
| "lib": ["es2021"], |
There was a problem hiding this comment.
My understanding is that: the values in the file only affect the documentation-generation process. Is that correct?
The transpilation of the .ts files in the src/ folder is not influenced by this file, yes?
There was a problem hiding this comment.
Yes, it only gets used to transpile ./typedoc.mts. Transpilation of the .ts files in src/ is not impacted.
ckeshava
left a comment
ckeshava
left a comment
There was a problem hiding this comment.
@Patel-Raj Can you also include the updated docs/ folder in this PR diff? I'd like to see if the documentation files are updated correctly.
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (9)
docs/assets/main.js (1)
4-6: Code formatting improvement in error messageThe multiline error message is now properly formatted with line breaks, improving readability in developer tools when errors occur.
🧰 Tools
🪛 Biome (1.9.4)
[error] 4-4: Other switch clauses can erroneously access this declaration.
Wrap the declaration in a block to restrict its access to the switch clause.The declaration is defined in this switch clause:
Unsafe fix: Wrap the declaration in a block.
(lint/correctness/noSwitchDeclarations)
[error] 4-4: Other switch clauses can erroneously access this declaration.
Wrap the declaration in a block to restrict its access to the switch clause.The declaration is defined in this switch clause:
Unsafe fix: Wrap the declaration in a block.
(lint/correctness/noSwitchDeclarations)
[error] 4-4: Other switch clauses can erroneously access this declaration.
Wrap the declaration in a block to restrict its access to the switch clause.The declaration is defined in this switch clause:
Unsafe fix: Wrap the declaration in a block.
(lint/correctness/noSwitchDeclarations)
[error] 4-4: Other switch clauses can erroneously access this declaration.
Wrap the declaration in a block to restrict its access to the switch clause.The declaration is defined in this switch clause:
Unsafe fix: Wrap the declaration in a block.
(lint/correctness/noSwitchDeclarations)
[error] 4-4: Other switch clauses can erroneously access this declaration.
Wrap the declaration in a block to restrict its access to the switch clause.The declaration is defined in this switch clause:
Unsafe fix: Wrap the declaration in a block.
(lint/correctness/noSwitchDeclarations)
[error] 4-4: Other switch clauses can erroneously access this declaration.
Wrap the declaration in a block to restrict its access to the switch clause.The declaration is defined in this switch clause:
Unsafe fix: Wrap the declaration in a block.
(lint/correctness/noSwitchDeclarations)
[error] 4-4: Other switch clauses can erroneously access this declaration.
Wrap the declaration in a block to restrict its access to the switch clause.The declaration is defined in this switch clause:
Unsafe fix: Wrap the declaration in a block.
(lint/correctness/noSwitchDeclarations)
[error] 5-5: Change to an optional chain.
Unsafe fix: Change to an optional chain.
(lint/complexity/useOptionalChain)
[error] 6-6: Change to an optional chain.
Unsafe fix: Change to an optional chain.
(lint/complexity/useOptionalChain)
[error] 4-4: Shouldn't redeclare 'E'. Consider to delete it or rename it.
'E' is defined here:
(lint/suspicious/noRedeclare)
[error] 4-4: Shouldn't redeclare 'E'. Consider to delete it or rename it.
'E' is defined here:
(lint/suspicious/noRedeclare)
[error] 4-4: Shouldn't redeclare 'E'. Consider to delete it or rename it.
'E' is defined here:
(lint/suspicious/noRedeclare)
[error] 4-4: Shouldn't redeclare 'E'. Consider to delete it or rename it.
'E' is defined here:
(lint/suspicious/noRedeclare)
[error] 4-4: Shouldn't redeclare 'E'. Consider to delete it or rename it.
'E' is defined here:
(lint/suspicious/noRedeclare)
[error] 4-4: Shouldn't redeclare 'E'. Consider to delete it or rename it.
'E' is defined here:
(lint/suspicious/noRedeclare)
[error] 4-4: Shouldn't redeclare 'E'. Consider to delete it or rename it.
'E' is defined here:
(lint/suspicious/noRedeclare)
[error] 4-4: Shouldn't redeclare 'l'. Consider to delete it or rename it.
'l' is defined here:
(lint/suspicious/noRedeclare)
[error] 4-4: Shouldn't redeclare 'c'. Consider to delete it or rename it.
'c' is defined here:
(lint/suspicious/noRedeclare)
[error] 4-4: Shouldn't redeclare 'i'. Consider to delete it or rename it.
'i' is defined here:
(lint/suspicious/noRedeclare)
[error] 4-4: Shouldn't redeclare 'l'. Consider to delete it or rename it.
'l' is defined here:
(lint/suspicious/noRedeclare)
[error] 4-4: Shouldn't redeclare 'f'. Consider to delete it or rename it.
'f' is defined here:
(lint/suspicious/noRedeclare)
[error] 4-4: Shouldn't redeclare 'y'. Consider to delete it or rename it.
'y' is defined here:
(lint/suspicious/noRedeclare)
[error] 4-4: Shouldn't redeclare 'f'. Consider to delete it or rename it.
'f' is defined here:
(lint/suspicious/noRedeclare)
[error] 4-4: Shouldn't redeclare 'y'. Consider to delete it or rename it.
'y' is defined here:
(lint/suspicious/noRedeclare)
[error] 4-4: Shouldn't redeclare 'l'. Consider to delete it or rename it.
'l' is defined here:
(lint/suspicious/noRedeclare)
[error] 4-4: Shouldn't redeclare 'y'. Consider to delete it or rename it.
'y' is defined here:
(lint/suspicious/noRedeclare)
[error] 4-4: Shouldn't redeclare 'l'. Consider to delete it or rename it.
'l' is defined here:
(lint/suspicious/noRedeclare)
[error] 4-4: Shouldn't redeclare 'Q'. Consider to delete it or rename it.
'Q' is defined here:
(lint/suspicious/noRedeclare)
[error] 4-4: Shouldn't redeclare 'l'. Consider to delete it or rename it.
'l' is defined here:
(lint/suspicious/noRedeclare)
[error] 4-4: Shouldn't redeclare 'T'. Consider to delete it or rename it.
'T' is defined here:
(lint/suspicious/noRedeclare)
[error] 4-4: Shouldn't redeclare 'c'. Consider to delete it or rename it.
'c' is defined here:
(lint/suspicious/noRedeclare)
[error] 4-4: Shouldn't redeclare 'd'. Consider to delete it or rename it.
'd' is defined here:
(lint/suspicious/noRedeclare)
[error] 4-4: Shouldn't redeclare 's'. Consider to delete it or rename it.
's' is defined here:
(lint/suspicious/noRedeclare)
[error] 4-4: Shouldn't redeclare 'r'. Consider to delete it or rename it.
'r' is defined here:
(lint/suspicious/noRedeclare)
[error] 4-4: Shouldn't redeclare 'r'. Consider to delete it or rename it.
'r' is defined here:
(lint/suspicious/noRedeclare)
[error] 4-4: Shouldn't redeclare 'i'. Consider to delete it or rename it.
'i' is defined here:
(lint/suspicious/noRedeclare)
[error] 4-4: Shouldn't redeclare 'i'. Consider to delete it or rename it.
'i' is defined here:
(lint/suspicious/noRedeclare)
[error] 4-4: Shouldn't redeclare 'i'. Consider to delete it or rename it.
'i' is defined here:
(lint/suspicious/noRedeclare)
[error] 4-4: Shouldn't redeclare 'i'. Consider to delete it or rename it.
'i' is defined here:
(lint/suspicious/noRedeclare)
[error] 4-4: Use a regular expression literal instead of the RegExp constructor.
Regular expression literals avoid some escaping required in a string literal, and are easier to analyze statically.
Safe fix: Use a literal notation instead.(lint/complexity/useRegexLiterals)
docs/enums/AccountSetTfFlags.html (2)
1-7: HTML Structural Improvements & Attribute Corrections
The modifications to the<html>tag now include adata-base=".."attribute and the<meta charset>tag is correctly defined withcharset. These changes improve consistency and standards compliance in the generated documentation.
8-14: Consistent Enumeration Member ID Updates
The enumeration members’ anchor links (e.g.#tfallowxrp,#tfdisallowxrp, etc.) have been updated to use lowercase IDs. Please verify that all downstream references (in scripts or style rules) are updated accordingly so that the navigation and linking continue to function as expected.docs/enums/AMMDepositFlags.html (1)
9-14: Addition & Consistency of Enumeration Members
A new enumeration member (tfTwoAssetIfEmpty) has been introduced, and the IDs for all members are consistently rendered in lowercase in their URL fragments. Please ensure that the changes here match the corresponding definitions in the TypeScript source (and any associated documentation links) to avoid broken references.docs/enums/NFTokenMintFlags.html (2)
1-12: Refinement of Enumeration Member Naming
The document now reflects updated enumeration member identifiers—such astfburnable,tfmutable,tfonlyxrp,tftransferable, andtftrustline—by using lowercase IDs in the anchors. Verify that the visible texts, which still include word-break tags (e.g.<wbr/>), align with the intended naming convention and that any external references use the new lowercase format.
13-23: New Member and Content Adjustments
The addition oftfMutable(as seen on line 9 in the index and in the detailed section) is now incorporated. Ensure that this new flag is correctly documented in the source code and the corresponding TypeScript model. Also, check if the visible text styling (which still features mixed case via<wbr/>) is intentional or if a fully lowercase display is preferred to match the IDs.docs/assets/style.css (3)
1-114: Enhanced Theming with New CSS Variables in @layer typedoc
A comprehensive set of new CSS custom properties (such as--light-color-icon-text,--light-color-comment-tag-text,--light-color-focus-outline, and their dark counterparts) has been added within the@layer typedocblock. This provides improved theming control for TypeDoc-generated pages. Consider adding inline comments describing the purpose of each new variable to aid future maintainability.
244-297: Theme Overrides in Root Selectors
The explicit definitions under:root[data-theme="light"]and:root[data-theme="dark"]correctly reassign the new CSS variables for specific themes. This setup improves clarity in style management. It might be useful to document these overrides in the project’s style guide for future reference.
298-1612: Overall CSS Styling & Responsiveness Enhancements
The remainder of the file maintains extensive styling rules for layout, typography, and responsive design. The integration of new animations, scrollbar styling, and responsive grid layouts is robust and cohesive. No critical issues were noted, but consider modularizing some of the CSS (if feasible) to simplify future updates.
📜 Review details
Configuration used: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (1)
docs/assets/icons.svgis excluded by!**/*.svg
📒 Files selected for processing (19)
docs/assets/hierarchy.js(1 hunks)docs/assets/highlight.css(6 hunks)docs/assets/icons.js(1 hunks)docs/assets/main.js(1 hunks)docs/assets/navigation.js(1 hunks)docs/assets/style.css(7 hunks)docs/enums/AMMClawbackFlags.html(1 hunks)docs/enums/AMMDepositFlags.html(1 hunks)docs/enums/AMMWithdrawFlags.html(1 hunks)docs/enums/AccountSetTfFlags.html(1 hunks)docs/enums/ECDSA.html(1 hunks)docs/enums/EnableAmendmentFlags.html(1 hunks)docs/enums/LedgerEntry.OfferFlags.html(1 hunks)docs/enums/LedgerEntry.SignerListFlags.html(1 hunks)docs/enums/MPTokenAuthorizeFlags.html(1 hunks)docs/enums/MPTokenIssuanceCreateFlags.html(1 hunks)docs/enums/MPTokenIssuanceSetFlags.html(1 hunks)docs/enums/NFTokenCreateOfferFlags.html(1 hunks)docs/enums/NFTokenMintFlags.html(1 hunks)
✅ Files skipped from review due to trivial changes (6)
- docs/assets/hierarchy.js
- docs/enums/MPTokenAuthorizeFlags.html
- docs/enums/MPTokenIssuanceSetFlags.html
- docs/enums/MPTokenIssuanceCreateFlags.html
- docs/assets/navigation.js
- docs/enums/AMMClawbackFlags.html
🧰 Additional context used
🪛 Biome (1.9.4)
docs/assets/main.js
[error] 4-4: Other switch clauses can erroneously access this declaration.
Wrap the declaration in a block to restrict its access to the switch clause.
The declaration is defined in this switch clause:
Unsafe fix: Wrap the declaration in a block.
(lint/correctness/noSwitchDeclarations)
[error] 4-4: Other switch clauses can erroneously access this declaration.
Wrap the declaration in a block to restrict its access to the switch clause.
The declaration is defined in this switch clause:
Unsafe fix: Wrap the declaration in a block.
(lint/correctness/noSwitchDeclarations)
[error] 4-4: Other switch clauses can erroneously access this declaration.
Wrap the declaration in a block to restrict its access to the switch clause.
The declaration is defined in this switch clause:
Unsafe fix: Wrap the declaration in a block.
(lint/correctness/noSwitchDeclarations)
[error] 4-4: Other switch clauses can erroneously access this declaration.
Wrap the declaration in a block to restrict its access to the switch clause.
The declaration is defined in this switch clause:
Unsafe fix: Wrap the declaration in a block.
(lint/correctness/noSwitchDeclarations)
[error] 4-4: Other switch clauses can erroneously access this declaration.
Wrap the declaration in a block to restrict its access to the switch clause.
The declaration is defined in this switch clause:
Unsafe fix: Wrap the declaration in a block.
(lint/correctness/noSwitchDeclarations)
[error] 4-4: Other switch clauses can erroneously access this declaration.
Wrap the declaration in a block to restrict its access to the switch clause.
The declaration is defined in this switch clause:
Unsafe fix: Wrap the declaration in a block.
(lint/correctness/noSwitchDeclarations)
[error] 4-4: Other switch clauses can erroneously access this declaration.
Wrap the declaration in a block to restrict its access to the switch clause.
The declaration is defined in this switch clause:
Unsafe fix: Wrap the declaration in a block.
(lint/correctness/noSwitchDeclarations)
[error] 5-5: Change to an optional chain.
Unsafe fix: Change to an optional chain.
(lint/complexity/useOptionalChain)
[error] 6-6: Change to an optional chain.
Unsafe fix: Change to an optional chain.
(lint/complexity/useOptionalChain)
[error] 4-4: Shouldn't redeclare 'E'. Consider to delete it or rename it.
'E' is defined here:
(lint/suspicious/noRedeclare)
[error] 4-4: Shouldn't redeclare 'E'. Consider to delete it or rename it.
'E' is defined here:
(lint/suspicious/noRedeclare)
[error] 4-4: Shouldn't redeclare 'E'. Consider to delete it or rename it.
'E' is defined here:
(lint/suspicious/noRedeclare)
[error] 4-4: Shouldn't redeclare 'E'. Consider to delete it or rename it.
'E' is defined here:
(lint/suspicious/noRedeclare)
[error] 4-4: Shouldn't redeclare 'E'. Consider to delete it or rename it.
'E' is defined here:
(lint/suspicious/noRedeclare)
[error] 4-4: Shouldn't redeclare 'E'. Consider to delete it or rename it.
'E' is defined here:
(lint/suspicious/noRedeclare)
[error] 4-4: Shouldn't redeclare 'E'. Consider to delete it or rename it.
'E' is defined here:
(lint/suspicious/noRedeclare)
[error] 4-4: Shouldn't redeclare 'l'. Consider to delete it or rename it.
'l' is defined here:
(lint/suspicious/noRedeclare)
[error] 4-4: Shouldn't redeclare 'c'. Consider to delete it or rename it.
'c' is defined here:
(lint/suspicious/noRedeclare)
[error] 4-4: Shouldn't redeclare 'i'. Consider to delete it or rename it.
'i' is defined here:
(lint/suspicious/noRedeclare)
[error] 4-4: Shouldn't redeclare 'l'. Consider to delete it or rename it.
'l' is defined here:
(lint/suspicious/noRedeclare)
[error] 4-4: Shouldn't redeclare 'f'. Consider to delete it or rename it.
'f' is defined here:
(lint/suspicious/noRedeclare)
[error] 4-4: Shouldn't redeclare 'y'. Consider to delete it or rename it.
'y' is defined here:
(lint/suspicious/noRedeclare)
[error] 4-4: Shouldn't redeclare 'f'. Consider to delete it or rename it.
'f' is defined here:
(lint/suspicious/noRedeclare)
[error] 4-4: Shouldn't redeclare 'y'. Consider to delete it or rename it.
'y' is defined here:
(lint/suspicious/noRedeclare)
[error] 4-4: Shouldn't redeclare 'l'. Consider to delete it or rename it.
'l' is defined here:
(lint/suspicious/noRedeclare)
[error] 4-4: Shouldn't redeclare 'y'. Consider to delete it or rename it.
'y' is defined here:
(lint/suspicious/noRedeclare)
[error] 4-4: Shouldn't redeclare 'l'. Consider to delete it or rename it.
'l' is defined here:
(lint/suspicious/noRedeclare)
[error] 4-4: Shouldn't redeclare 'Q'. Consider to delete it or rename it.
'Q' is defined here:
(lint/suspicious/noRedeclare)
[error] 4-4: Shouldn't redeclare 'l'. Consider to delete it or rename it.
'l' is defined here:
(lint/suspicious/noRedeclare)
[error] 4-4: Shouldn't redeclare 'T'. Consider to delete it or rename it.
'T' is defined here:
(lint/suspicious/noRedeclare)
[error] 4-4: Shouldn't redeclare 'c'. Consider to delete it or rename it.
'c' is defined here:
(lint/suspicious/noRedeclare)
[error] 4-4: Shouldn't redeclare 'd'. Consider to delete it or rename it.
'd' is defined here:
(lint/suspicious/noRedeclare)
[error] 4-4: Shouldn't redeclare 's'. Consider to delete it or rename it.
's' is defined here:
(lint/suspicious/noRedeclare)
[error] 4-4: Shouldn't redeclare 'r'. Consider to delete it or rename it.
'r' is defined here:
(lint/suspicious/noRedeclare)
[error] 4-4: Shouldn't redeclare 'r'. Consider to delete it or rename it.
'r' is defined here:
(lint/suspicious/noRedeclare)
[error] 4-4: Shouldn't redeclare 'i'. Consider to delete it or rename it.
'i' is defined here:
(lint/suspicious/noRedeclare)
[error] 4-4: Shouldn't redeclare 'i'. Consider to delete it or rename it.
'i' is defined here:
(lint/suspicious/noRedeclare)
[error] 4-4: Shouldn't redeclare 'i'. Consider to delete it or rename it.
'i' is defined here:
(lint/suspicious/noRedeclare)
[error] 4-4: Shouldn't redeclare 'i'. Consider to delete it or rename it.
'i' is defined here:
(lint/suspicious/noRedeclare)
[error] 4-4: Use a regular expression literal instead of the RegExp constructor.
Regular expression literals avoid some escaping required in a string literal, and are easier to analyze statically.
Safe fix: Use a literal notation instead.
(lint/complexity/useRegexLiterals)
⏰ Context from checks skipped due to timeout of 90000ms (7)
- GitHub Check: snippets (22.x)
- GitHub Check: snippets (18.x)
- GitHub Check: integration (22.x)
- GitHub Check: integration (18.x)
- GitHub Check: snippets (20.x)
- GitHub Check: browser (18.x)
- GitHub Check: integration (20.x)
🔇 Additional comments (15)
docs/enums/ECDSA.html (1)
1-9: Documentation HTML structure updated for TypeDoc 0.27 compatibilityThese changes to the HTML structure are part of the fix for the docgen issue mentioned in the PR objectives. The updates include:
- Adding
data-baseattribute to the html tag- Fixing the charset meta tag from
charSettocharset- Adding new script tags for icons, navigation, and hierarchy
- Implementing improved page loading with timeout
The changes align with TypeDoc 0.27's ESM requirements, which was the root cause of the documentation generation failure.
docs/enums/AMMWithdrawFlags.html (1)
1-15: Updated HTML structure consistent with TypeDoc 0.27 outputThe changes to this documentation file are consistent with the updates seen in other enum HTML files, properly implementing the new TypeDoc 0.27 structure with improved script handling and metadata.
docs/enums/LedgerEntry.SignerListFlags.html (1)
1-8: Documentation structure modernized for ESM compatibilityThese changes follow the same pattern as other documentation files, properly implementing the TypeDoc 0.27 output format with proper charset declaration and script loading.
docs/enums/EnableAmendmentFlags.html (1)
1-12: Documentation HTML updated for TypeDoc ESM complianceThe structure updates in this file match the changes in other documentation files, correctly implementing the TypeDoc 0.27 output format with improved script handling and DOM rendering.
docs/assets/icons.js (1)
1-18: New icon handling system supports TypeDoc 0.27 functionalityThis new JavaScript file is part of the TypeDoc 0.27 output that now uses ES Modules. It provides SVG icons for the documentation interface and handles file protocol references properly.
The implementation is well-structured with:
- Proper event handling for DOM loading
- SVG icon definitions
- Special handling for file protocol URLs
This supports the PR's goal of fixing the docgen issue by being compatible with TypeDoc 0.27's ESM requirements.
docs/assets/highlight.css (2)
24-27: Color variable additions for improved syntax highlightingThese changes update existing color variables and add new ones for syntax highlighting in the documentation. The previous error color (#CD3131 / #F44747) has been reassigned to the new
--hl-12variables, while--hl-11now represents black/light gray text.
45-45: New highlight class properly implemented across all theme contextsThe new highlight variable is correctly implemented in all theme contexts (media queries and data-theme attributes) to ensure consistent highlighting regardless of the user's theme preference.
Also applies to: 62-62, 79-79, 96-96, 112-112
docs/enums/NFTokenCreateOfferFlags.html (2)
1-7: HTML structure modernized with data-base attribute and proper charsetThe HTML structure has been updated with a
data-baseattribute for proper path resolution and the charset meta tag has been corrected fromcharSetto the standardcharsetattribute name.
9-11: Documentation clarity improvement for NFTokenCreateOfferFlagsThe documentation for
tfSellNFTokenhas been improved by adding the clarification that when the flag is not set, it represents a buy offer. This enhances the understanding of the enum's behavior.docs/enums/LedgerEntry.OfferFlags.html (2)
1-7: HTML structure modernized with appropriate base path and charsetSimilar to other HTML files, the structure has been updated with a
data-baseattribute and corrected charset meta tag, ensuring consistent documentation rendering.
8-9: Improved enum member identifiers with consistent casingThe enum member links use lowercase identifiers in the URL fragment (lsfpassive, lsfsell) while maintaining proper display casing in the UI, improving consistency across the documentation.
docs/assets/main.js (2)
2-2: Added internationalization support through translations objectA global translations object has been added to support multiple languages in the UI. This improves accessibility and user experience for non-English speakers.
3-3: Assignment pattern prevents duplicate strict mode declarationsThe use of "use strict" within the IIFE after the translations declaration ensures proper strict mode activation without duplicating the declaration.
docs/enums/AMMDepositFlags.html (1)
1-7: Standardization of Document Structure
Similar to the AccountSetTfFlags file, the<html>tag now carries thedata-base=".."attribute and the<meta charset>tag is corrected. These improvements help standardize the HTML structure across your docs.docs/assets/style.css (1)
116-234: Media Query Adjustments for Color Schemes
The media queries forprefers-color-scheme: lightandprefers-color-scheme: darknow leverage the newly defined variables, ensuring the proper application of color themes. These changes appear well organized; just verify that all fallback values work across browsers as expected.
|
@ckeshava I have updated the docs as well. Can you please have a look? |
| (function(w,d,s,l,i){w[l]=w[l]||[];w[l].push({'gtm.start': | ||
| new Date().getTime(),event:'gtm.js'});var f=d.getElementsByTagName(s)[0], | ||
| j=d.createElement(s),dl=l!='dataLayer'?'&l='+l:'';j.async=true;j.src= | ||
| 'https://www.googletagmanager.com/gtm.js?id='+i+dl;f.parentNode.insertBefore(j,f); | ||
| })(window,document,'script','dataLayer','GTM-M7HKJJ3'); | ||
| </script><meta http-equiv="x-ua-compatible" content="IE=edge"/><title>decodeAccountPublic | xrpl</title><meta name="description" content="Documentation for xrpl"/><meta name="viewport" content="width=device-width, initial-scale=1"/><link rel="stylesheet" href="../assets/style.css"/><link rel="stylesheet" href="../assets/highlight.css"/><script defer src="../assets/main.js"></script><script async src="../assets/search.js" id="tsd-search-script"></script></head><body><noscript><iframe src="https://www.googletagmanager.com/ns.html?id=GTM-M7HKJJ3" height=0 width=0 style="display:none;visibility:hidden"></iframe></noscript><script>document.documentElement.dataset.theme = localStorage.getItem("tsd-theme") || "os"</script><header class="tsd-page-toolbar"> |
There was a problem hiding this comment.
I'm confused by the empty rendition of some of the docs pages
The code for this method is found here
Do you know why this happens?
There was a problem hiding this comment.
The function is not directly accessible from the index.html page once I clear the cache. To me it seems like only the exports from xrpl package are generated on the docs site.
There was a problem hiding this comment.
How did you clear the cache? npm cache clean ?
There was a problem hiding this comment.
I cleared the browser cache.
| @@ -4,10 +4,10 @@ | |||
| "jsx": "react", | |||
There was a problem hiding this comment.
Regarding the line above: "outDir": "./dist/docs", -- is this the location of the generated documentation files?
There was a problem hiding this comment.
This is the location of the transpiled typedoc.mjs plugin file that gets used by the Typedoc while generating the actual documentations.
|
I observe several warnings during the generation of documentation. Can you please create a ticket for fixing those warnings on Github issues? That will help us track it. I observe these warnings on my computer: |
4 participants
High Level Overview of Change
Used .mts file extension for the Typedoc plugin to emit ES Modules (ESM) by default, since TypeDoc 0.27 is ESM only.
TypeStrong/typedoc#2870
Context of Change
docgen command to generate docs was failing since the typedoc.tsx to typedoc.js transpilation was resulted in require calls to import from ECMAScript module typedoc.
Type of Change
Did you update HISTORY.md?
Test Plan
Before the fix: "npm run docgen" fails to generate the docs
After this fix: "npm run docgen" runs and produces output in /docs directory.
Future Tasks
Once this PR is approved and merged, docs needs to be generated and merged in the main branch.