Published updates - #161
Conversation
Summary by CodeRabbit
WalkthroughUpdated package.json dependency placeholders to concrete 4.x versions, bumped content package patch version, updated content README links/examples to point at TypeScript sources and optional params, and added a JSDoc comment for CrafterConfig.flatten. No behavioral or type changes. Changes
Estimated code review effort🎯 2 (Simple) | ⏱️ ~10 minutes Possibly related PRs
Suggested reviewers
✨ Finishing Touches🧪 Generate unit tests
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. 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
SupportNeed help? Create a ticket on our support page for assistance with any issues or questions. CodeRabbit Commands (Invoked using PR/Issue comments)Type Other keywords and placeholders
CodeRabbit Configuration File (
|
![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
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (2)
packages/content/README.md (2)
280-289: Fix example to use getNavTree (typo currently calling getTree)Use the correct function and align variable naming.
- import { getTree } from '@craftercms/content'; + import { getNavTree } from '@craftercms/content'; @@ - getTree('/site/website', 3, { site: 'editorial' }).subscribe((tree) => { - console.log(tree); + getNavTree('/site/website', 3, '', { site: 'editorial' }).subscribe((navTree) => { + console.log(navTree); }); @@ - getTree('/site/website', 3).subscribe((tree) => { - console.log(tree); + getNavTree('/site/website', 3).subscribe((navTree) => { + console.log(navTree); });
315-323: Fix example labeling/arguments for getNavBreadcrumb“Example 1: Config supplied inline” currently omits config. Pass config or change the label. Suggest passing config and adjusting Example 2 accordingly.
- // Example 1: Config supplied inline - getNavBreadcrumb('/site/website').subscribe((navBreadcrumb) => { + // Example 1: Config supplied inline + getNavBreadcrumb('/site/website', '', { site: 'editorial' }).subscribe((navBreadcrumb) => { console.log(navBreadcrumb); }); @@ - // Example 2: Services pre-configured (see "Usage" section above), config param omitted. - getNavBreadcrumb('/site/website').subscribe((navBreadcrumb) => { + // Example 2: Services pre-configured (see "Usage" section above), config param omitted. + getNavBreadcrumb('/site/website').subscribe((navBreadcrumb) => { console.log(navBreadcrumb); });
🧹 Nitpick comments (8)
packages/content/README.md (8)
153-156: Fix phrasing: “Default value in here” → clearer link textImprove readability of the config row.
-| config | Crafter configuration. Optional. Default value in [here](../models/src/CrafterConfig.ts). | +| config | Crafter configuration. Optional. Defaults: see [CrafterConfig](../models/src/CrafterConfig.ts). |
197-201: Unify config phrasing in tablesApply the same clearer wording here.
-| config | Crafter configuration. Optional. Default value in [here](../models/src/CrafterConfig.ts). | +| config | Crafter configuration. Optional. Defaults: see [CrafterConfig](../models/src/CrafterConfig.ts). |
229-234: Unify config phrasing; minor clarity on depthTweak wording and keep consistency.
-| depth | Amount of levels to include. Optional. Default is `1` | -| config | Crafter configuration. Optional. Default value in [here](../models/src/CrafterConfig.ts). | +| depth | Number of levels to include. Optional. Default: `1` | +| config | Crafter configuration. Optional. Defaults: see [CrafterConfig](../models/src/CrafterConfig.ts). |
264-270: Unify config phrasing; minor clarity on depth/currentPageUrlSmall wording tweaks for consistency.
-| depth | Amount of levels to include. Optional. Default is `1` | +| depth | Number of levels to include. Optional. Default: `1` | -| currentPageUrl | The URL of the current page. Optional. Default is `''` | +| currentPageUrl | The URL of the current page. Optional. Default: `''` | -| config | Crafter configuration. Optional. Default value in [here](../models/src/CrafterConfig.ts). | +| config | Crafter configuration. Optional. Defaults: see [CrafterConfig](../models/src/CrafterConfig.ts). |
298-303: Grammar/formatting: capitalize and unify config phrasingAlso resolves the LanguageTool flags.
-| path | The folder’s path | -| root | the root URL, basically the starting point of the breadcrumb. Optional. Default is `''` | -| config | Crafter configuration. Optional. Default value in [here](../models/src/CrafterConfig.ts). | +| path | The folder’s path | +| root | The root URL; starting point of the breadcrumb. Optional. Default: `''` | +| config | Crafter configuration. Optional. Defaults: see [CrafterConfig](../models/src/CrafterConfig.ts). |
306-306: Pluralize return descriptionMinor wording improvement.
-[NavigationItem](../models/src/NavigationItem.ts)[] - List of NavigationItem from the content store +[NavigationItem](../models/src/NavigationItem.ts)[] - List of navigation items from the content store
331-336: Unify config phrasing in Transform sectionKeep consistent wording across tables.
-| config | Crafter configuration. Optional. Default value in [here](../models/src/CrafterConfig.ts). | +| config | Crafter configuration. Optional. Defaults: see [CrafterConfig](../models/src/CrafterConfig.ts). |
343-357: Optional: Align function name in bullet with examples (transform vs urlTransform)Bullet shows
transform(...)but first example usesurlTransform(...). If both APIs exist, consider documenting both signatures or standardizing the naming in this section.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
💡 Knowledge Base configuration:
- MCP integration is disabled by default for public repositories
- Jira integration is disabled by default for public repositories
- Linear integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
📒 Files selected for processing (2)
packages/content/README.md(6 hunks)packages/content/package.json(1 hunks)
✅ Files skipped from review due to trivial changes (1)
- packages/content/package.json
🧰 Additional context used
🪛 LanguageTool
packages/content/README.md
[grammar] ~298-~298: There might be a mistake here.
Context: ... | | -------------- |:---------------------...
(QB_NEW_EN)
[grammar] ~299-~299: There might be a mistake here.
Context: ...--------------------------------------:| | path | ...
(QB_NEW_EN)
[grammar] ~300-~300: There might be a mistake here.
Context: ...th | | root | the root URL, basica...
(QB_NEW_EN)
🔇 Additional comments (4)
packages/content/README.md (4)
160-160: LGTM: Return type link updated correctlyLinking Item to the TS source is helpful and accurate.
204-204: LGTM: Children return type“Item[]” with the link on Item is fine.
237-237: Verify: getTree return shape (single Item vs tree/array)Doc says “complete Item hierarchy”, but return shows a single Item. Should this be Item[] or a tree structure?
Proposed if it should be an array:
-[Item](../models/src/item.ts) - from the content store +[Item](../models/src/item.ts)[] - Item hierarchy from the content store
273-273: Verify: getNavTree return type; example below uses getTreeReturn shows a single NavigationItem, but API returns a navigation tree; likely an array or root node structure. Also, the example below imports/calls getTree instead of getNavTree.
If it returns an array, apply:
-[NavigationItem](../models/src/NavigationItem.ts) - from the content store +[NavigationItem](../models/src/NavigationItem.ts)[] - Navigation tree from the content store
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Nitpick comments (1)
packages/content/README.md (1)
298-303: Breadcrumb params: capitalize sentence, tighten wording; table alignmentMinor language and consistency fixes. Also the text states root is optional; ensure the signature above reflects that (root?: string).
Apply this diff:
-| path | The folder’s path | -| root | the root URL, basically the starting point of the breadcrumb. Optional. Default is `''` | -| config | Crafter configuration. Optional. Default value in [here](../models/src/CrafterConfig.ts). | +| path | The folder’s path | +| root | The root URL (starting point of the breadcrumb). Optional. Default is `''`. | +| config | Crafter configuration. Optional. Default in [CrafterConfig.ts](../models/src/CrafterConfig.ts). |
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
💡 Knowledge Base configuration:
- MCP integration is disabled by default for public repositories
- Jira integration is disabled by default for public repositories
- Linear integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
📒 Files selected for processing (1)
packages/content/README.md(7 hunks)
🧰 Additional context used
🪛 LanguageTool
packages/content/README.md
[grammar] ~298-~298: There might be a mistake here.
Context: ... | | -------------- |:---------------------...
(QB_NEW_EN)
[grammar] ~299-~299: There might be a mistake here.
Context: ...--------------------------------------:| | path | ...
(QB_NEW_EN)
[grammar] ~300-~300: There might be a mistake here.
Context: ...th | | root | the root URL, basica...
(QB_NEW_EN)
🔇 Additional comments (7)
packages/content/README.md (7)
153-156: Good update: link CrafterConfig default to TS sourceLinking to ../models/src/CrafterConfig.ts is accurate and helpful.
160-160: Return type link fixedUsing ../models/src/item.ts is correct.
197-201: Consistent config linkParameters table now points to the TS source for defaults; looks good.
204-204: Return type link fixedUsing ../models/src/item.ts[] is correct.
229-234: Params table consistencyDepth/config docs read well and link to TS source.
306-306: Return type link fixedNavigationItem link to TS source looks good.
331-336: Config link correctionPointing to CrafterConfig.ts for defaults is consistent across sections.
There was a problem hiding this comment.
Actionable comments posted: 3
♻️ Duplicate comments (2)
packages/content/README.md (2)
262-262: Resolved: getNavTree signature now reflects optional args and arg orderThis addresses the earlier confusion around currentPageUrl and config.
280-284: Resolved: Example now passes empty currentPageUrl placeholderThe example call aligns with the updated signature and prevents arg misplacement.
🧹 Nitpick comments (3)
packages/content/README.md (3)
153-156: Nit: Clarify phrasing for config default reference“Default value in here” is awkward. Prefer a direct reference to the file name.
Apply:
-| config | Crafter configuration. Optional. Default value in [here](../models/src/CrafterConfig.ts). | +| config | Crafter configuration. Optional. Default in [CrafterConfig.ts](../models/src/CrafterConfig.ts). |
197-201: Nit: Same config phrasing improvement as aboveRepeat the clearer wording for consistency.
-| config | Crafter configuration. Optional. Default value in [here](../models/src/CrafterConfig.ts). | +| config | Crafter configuration. Optional. Default in [CrafterConfig.ts](../models/src/CrafterConfig.ts). |
264-270: Nit: Improve config default phrasingKeep wording consistent across sections.
-| config | Crafter configuration. Optional. Default value in [here](../models/src/CrafterConfig.ts). | +| config | Crafter configuration. Optional. Default in [CrafterConfig.ts](../models/src/CrafterConfig.ts). |
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
💡 Knowledge Base configuration:
- MCP integration is disabled by default for public repositories
- Jira integration is disabled by default for public repositories
- Linear integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
📒 Files selected for processing (1)
packages/content/README.md(7 hunks)
🧰 Additional context used
🪛 LanguageTool
packages/content/README.md
[grammar] ~298-~298: There might be a mistake here.
Context: ... | | -------------- |:---------------------...
(QB_NEW_EN)
[grammar] ~299-~299: There might be a mistake here.
Context: ...--------------------------------------:| | path | ...
(QB_NEW_EN)
[grammar] ~300-~300: There might be a mistake here.
Context: ...th | | root | the root URL, basica...
(QB_NEW_EN)
🔇 Additional comments (7)
packages/content/README.md (7)
160-160: LGTM: Type link updatedItem now correctly links to the TS source path.
204-204: LGTM: Return type linkArray Item link points to models/src/item.ts as expected.
237-237: LGTM: Return type linkReturn type references models/src/item.ts correctly.
273-273: LGTM: Return type linkNavigationItem link points to the TS source file.
287-288: LGTM: Example comment clarifies omitted currentPageUrl defaultClear and consistent with the signature.
306-306: LGTM: Return type linkBreadcrumb return type links to NavigationItem TS source.
316-316: LGTM: Example passes explicit rootMatches the (intended) optional root param and shows usage clearly.
| | Parameters | | | ||
| | ------------- |:-----------------------------------------------------------------------------------------:| | ||
| | path | The folder’s path | | ||
| | depth | Amount of levels to include. Optional. Default is `1` | | ||
| | config | Crafter configuration. Optional. Default value in [here](../models/src/CrafterConfig.ts). | | ||
|
|
There was a problem hiding this comment.
🛠️ Refactor suggestion
Signature/table mismatch for getTree: make depth and config optional or fix table
The table marks depth and config as optional, but the signature above shows both required. Align these.
Proposed fix:
-`getTree(path: string, depth: number, config: CrafterConfig)`
+`getTree(path: string, depth?: number, config?: CrafterConfig)`And keep the table as-is (with optional), plus the config phrasing nit:
-| config | Crafter configuration. Optional. Default value in [here](../models/src/CrafterConfig.ts). |
+| config | Crafter configuration. Optional. Default in [CrafterConfig.ts](../models/src/CrafterConfig.ts). |If the API truly requires both, then instead change the table to remove “Optional” and the default note.
📝 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.
| | Parameters | | | |
| | ------------- |:-----------------------------------------------------------------------------------------:| | |
| | path | The folder’s path | | |
| | depth | Amount of levels to include. Optional. Default is `1` | | |
| | config | Crafter configuration. Optional. Default value in [here](../models/src/CrafterConfig.ts). | | |
| `getTree(path: string, depth?: number, config?: CrafterConfig)` | |
| | Parameters | | | |
| | ------------- |:-----------------------------------------------------------------------------------------------:| | |
| | path | The folder’s path | | |
| | depth | Amount of levels to include. Optional. Default is `1` | | |
| | config | Crafter configuration. Optional. Default in [CrafterConfig.ts](../models/src/CrafterConfig.ts). | |
🤖 Prompt for AI Agents
In packages/content/README.md around lines 229-234, the README table marks
getTree's depth and config parameters as optional but the function signature
above shows them required; either make the function signature match the table by
changing the getTree signature to accept optional parameters (e.g., depth?:
number with default 1 and config?: CrafterConfig with the existing default) or
update the table to show them as required (remove "Optional" and default note).
If choosing to make parameters optional, also update the config cell wording to
"Crafter configuration. Optional. Default value: see
../models/src/CrafterConfig.ts" so the documentation and signature are aligned.
| | Parameters | | | ||
| | -------------- |:-----------------------------------------------------------------------------------------:| | ||
| | path | The folder’s path | | ||
| | root | the root URL, basically the starting point of the breadcrumb. Optional. Default is `''` | | ||
| | config | Crafter configuration. Optional. Default value in [here](../models/src/CrafterConfig.ts). | | ||
|
|
There was a problem hiding this comment.
Signature/table mismatch for getNavBreadcrumb + minor grammar
Root and config are shown as optional in the table, but the signature above shows them required. Also, tighten the root description.
Apply:
-`getNavBreadcrumb(path: string, root: string, config: CrafterConfig)`
+`getNavBreadcrumb(path: string, root?: string, config?: CrafterConfig)`And refine table wording:
-| root | the root URL, basically the starting point of the breadcrumb. Optional. Default is `''` |
+| root | The root URL; starting point of the breadcrumb. Optional. Default is `''`. |
-| config | Crafter configuration. Optional. Default value in [here](../models/src/CrafterConfig.ts). |
+| config | Crafter configuration. Optional. Default in [CrafterConfig.ts](../models/src/CrafterConfig.ts). |📝 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.
| | Parameters | | | |
| | -------------- |:-----------------------------------------------------------------------------------------:| | |
| | path | The folder’s path | | |
| | root | the root URL, basically the starting point of the breadcrumb. Optional. Default is `''` | | |
| | config | Crafter configuration. Optional. Default value in [here](../models/src/CrafterConfig.ts). | | |
| `getNavBreadcrumb(path: string, root?: string, config?: CrafterConfig)` | |
| | Parameters | | | |
| | -------------- |:-----------------------------------------------------------------------------------------:| | |
| | path | The folder’s path | | |
| | root | The root URL; starting point of the breadcrumb. Optional. Default is `''`. | | |
| | config | Crafter configuration. Optional. Default in [CrafterConfig.ts](../models/src/CrafterConfig.ts). | |
🧰 Tools
🪛 LanguageTool
[grammar] ~298-~298: There might be a mistake here.
Context: ... | | -------------- |:---------------------...
(QB_NEW_EN)
[grammar] ~299-~299: There might be a mistake here.
Context: ...--------------------------------------:| | path | ...
(QB_NEW_EN)
[grammar] ~300-~300: There might be a mistake here.
Context: ...th | | root | the root URL, basica...
(QB_NEW_EN)
| | Parameters | | | ||
| | --------------- |:-----------------------------------------------------------------------------------------:| | ||
| | transformerName | Name of the transformer to apply | | ||
| | path | URL that will be transformed | | ||
| | config | Crafter configuration. Optional. Default value in [here](../models/src/CrafterConfig.ts). | | ||
|
|
There was a problem hiding this comment.
🛠️ Refactor suggestion
Signature/table mismatch for transform config optionality
Table marks config optional; signature above shows it required. Align these and fix config phrasing.
- - `transform(transformerName: string, path: string, config: CrafterConfig)`
+ - `transform(transformerName: string, path: string, config?: CrafterConfig)` And:
-| config | Crafter configuration. Optional. Default value in [here](../models/src/CrafterConfig.ts). |
+| config | Crafter configuration. Optional. Default in [CrafterConfig.ts](../models/src/CrafterConfig.ts). |📝 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.
| | Parameters | | | |
| | --------------- |:-----------------------------------------------------------------------------------------:| | |
| | transformerName | Name of the transformer to apply | | |
| | path | URL that will be transformed | | |
| | config | Crafter configuration. Optional. Default value in [here](../models/src/CrafterConfig.ts). | | |
| `transform(transformerName: string, path: string, config?: CrafterConfig)` | |
| | Parameters | | | |
| | --------------- |:-----------------------------------------------------------------------------------------:| | |
| | transformerName | Name of the transformer to apply | | |
| | path | URL that will be transformed | | |
| | config | Crafter configuration. Optional. Default in [CrafterConfig.ts](../models/src/CrafterConfig.ts). | |
🤖 Prompt for AI Agents
In packages/content/README.md around lines 331-336, the parameter table marks
"config" as optional but the function signature shown earlier documents it as
required; make them consistent by updating the signature to show config as
optional (e.g., add the optional marker or provide a default) and reword the
table entry to a concise phrase such as "Crafter configuration — optional.
Defaults to the value in ../models/src/CrafterConfig.ts." Ensure both signature
and table use the same optional notation and identical phrasing for clarity.
|
@rart I turned this into a draft until you're done with the rabbit. |
2 participants
craftersoftware/craftercms#8054