Conversation
📝 WalkthroughWalkthroughDependency/version updates and render-helper tweaks: type export added; markdown-to-HTML behavior adjusted (new block-tag fixer and 3Speak embed changes); render helper usage swapped to Changes
Sequence Diagram(s)(Skipped — changes are refactors, parsing/regex updates, type exports, and tests; no new multi-component sequential flow requiring visualization.) Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Possibly related PRs
Poem
🚥 Pre-merge checks | ✅ 1 | ❌ 2❌ Failed checks (1 warning, 1 inconclusive)
✅ Passed checks (1 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing touches
📜 Recent review detailsConfiguration used: Organization UI Review profile: CHILL Plan: Pro ⛔ Files ignored due to path filters (6)
📒 Files selected for processing (8)
🚧 Files skipped from review as they are similar to previous changes (2)
🧰 Additional context used🧬 Code graph analysis (3)packages/render-helper/src/methods/markdown-to-html.method.spec.ts (1)
packages/render-helper/src/methods/iframe.method.spec.ts (1)
packages/render-helper/src/methods/iframe.method.ts (3)
🪛 ast-grep (0.40.5)packages/render-helper/src/methods/markdown-to-html.method.ts[warning] 43-43: Regular expression constructed from variable input detected. This can lead to Regular Expression Denial of Service (ReDoS) attacks if the variable contains malicious patterns. Use libraries like 'recheck' to validate regex safety or use static patterns. (regexp-from-variable) [warning] 48-48: Regular expression constructed from variable input detected. This can lead to Regular Expression Denial of Service (ReDoS) attacks if the variable contains malicious patterns. Use libraries like 'recheck' to validate regex safety or use static patterns. (regexp-from-variable) [warning] 53-53: Regular expression constructed from variable input detected. This can lead to Regular Expression Denial of Service (ReDoS) attacks if the variable contains malicious patterns. Use libraries like 'recheck' to validate regex safety or use static patterns. (regexp-from-variable) [warning] 57-57: Regular expression constructed from variable input detected. This can lead to Regular Expression Denial of Service (ReDoS) attacks if the variable contains malicious patterns. Use libraries like 'recheck' to validate regex safety or use static patterns. (regexp-from-variable) 🔇 Additional comments (7)
✏️ Tip: You can disable this entire section by setting 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 |
![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 (3)
apps/web/src/features/market/market-swap-form/api/swapping.tsx (2)
113-121: Potential method/auth mismatch whenshouldUseHiveAuthreturns true butloginTypediffers.The comment states this avoids method/auth mismatch, but
getSdkAuthContext(line 17-18 in the relevant snippet) sets up HiveAuth broadcast when eitherloginType === "hiveauth"orshouldUseHiveAuth(user.username)is true.If
shouldUseHiveAuth(username)returns true butuser.loginTypeis not"hiveauth", then:
authwill be configured with HiveAuth broadcastmethodwill be"keychain"(line 115)This creates the exact mismatch the comment claims to avoid.
Suggested fix to align method selection with auth setup
// Use actual loginType instead of shouldUseHiveAuth to avoid method/auth mismatch - const method = user?.loginType === "hiveauth" ? "hiveauth" : "keychain"; + const method = user?.loginType === "hiveauth" || (user && shouldUseHiveAuth(user.username)) ? "hiveauth" : "keychain";Alternatively, if the intent is to stop using
shouldUseHiveAuthfor this flow, thengetSdkAuthContextshould also be updated to not use it when called from here, or a different auth context construction should be used.
13-13: Remove unused importshouldUseHiveAuth.This import is no longer used after the refactor that changed line 114 to use
loginTypedirectly. The comment in the code explicitly documents this: "Use actual loginType instead of shouldUseHiveAuth to avoid method/auth mismatch."packages/renderer/src/lib/components/extensions/wave-like-post-extension.tsx (1)
112-115:hydrateRootis intended for hydrating server-rendered HTML, not for rendering into empty containers.
hydrateRootexpects the container to already have server-rendered HTML that matches the React component tree. Here you're creating a new empty<div>and rendering into it, which is the use case forcreateRoot.🐛 Proposed fix
-import { hydrateRoot } from "react-dom/client"; +import { createRoot } from "react-dom/client";- hydrateRoot( + createRoot(container).render( - container, <WaveLikePostRenderer link={element.getAttribute("href") ?? ""} />, );
🧹 Nitpick comments (3)
apps/web/src/app/decks/_components/columns/deck-whats-new-column.tsx (1)
44-45: Consider logging or handling fetch errors.The empty
catchblock silently swallows errors. Consider logging the error for debugging purposes.Suggested improvement
} catch (e) { + console.error("Failed to fetch releases:", e); } finally {packages/renderer/src/lib/api/hive.api.ts (1)
1-7: Consider importingEntryfrom@ecency/render-helperto avoid duplication.This local interface duplicates the canonical
Entryinterface frompackages/render-helper/src/types/entry.interface.ts, which is now exported publicly from the package (seepackages/render-helper/src/index.tsline 20). The same duplicate interface also appears inwave-like-post-extension.tsx.Importing from the shared package would reduce maintenance burden and ensure type consistency.
♻️ Suggested refactor
-interface Entry { - author?: string; - permlink?: string; - last_update?: string; - body: any; - json_metadata?: any; -} +import type { Entry } from "@ecency/render-helper";packages/renderer/src/lib/components/extensions/wave-like-post-extension.tsx (1)
11-17: Same duplication ashive.api.ts— consider a shared import.This is the same
Entryinterface defined locally inhive.api.ts. Since@ecency/render-helpernow exportsEntrypublicly, both files could import from the shared package to maintain a single source of truth.♻️ Suggested refactor
-interface Entry { - author?: string; - permlink?: string; - last_update?: string; - body: any; - json_metadata?: any; -} +import type { Entry } from "@ecency/render-helper";
📜 Review details
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (3)
packages/render-helper/dist/browser/index.d.tsis excluded by!**/dist/**packages/renderer/dist/lib/api/hive.api.d.tsis excluded by!**/dist/**pnpm-lock.yamlis excluded by!**/pnpm-lock.yaml
📒 Files selected for processing (9)
apps/web/package.jsonapps/web/public/sw.jsapps/web/src/app/decks/_components/columns/deck-whats-new-column.tsxapps/web/src/features/market/market-swap-form/api/swapping.tsxpackages/render-helper/src/index.tspackages/render-helper/src/types/entry.interface.tspackages/renderer/package.jsonpackages/renderer/src/lib/api/hive.api.tspackages/renderer/src/lib/components/extensions/wave-like-post-extension.tsx
🧰 Additional context used
🧬 Code graph analysis (3)
packages/renderer/src/lib/components/extensions/wave-like-post-extension.tsx (3)
packages/render-helper/src/index.ts (1)
Entry(21-21)packages/render-helper/src/types/entry.interface.ts (1)
Entry(4-10)packages/render-helper/dist/browser/index.d.ts (1)
Entry(35-35)
apps/web/src/features/market/market-swap-form/api/swapping.tsx (1)
apps/web/src/utils/sdk-auth.ts (1)
getSdkAuthContext(6-46)
packages/renderer/src/lib/api/hive.api.ts (3)
packages/render-helper/src/index.ts (1)
Entry(21-21)packages/render-helper/src/types/entry.interface.ts (1)
Entry(4-10)packages/render-helper/dist/browser/index.d.ts (1)
Entry(35-35)
🪛 Biome (2.1.2)
apps/web/src/app/decks/_components/columns/deck-whats-new-column.tsx
[error] 82-82: Avoid passing content using the dangerouslySetInnerHTML prop.
Setting content using code can expose users to cross-site scripting (XSS) attacks
(lint/security/noDangerouslySetInnerHtml)
🔇 Additional comments (9)
apps/web/src/features/market/market-swap-form/api/swapping.tsx (1)
89-93: Auth context construction looks good.The two-step approach of first resolving the user, then building the auth context is cleaner and more explicit than chaining.
packages/render-helper/src/types/entry.interface.ts (1)
1-10: LGTM on the JSDoc addition.The documentation clarifies the interface purpose. Consider using
stringforbodyandstring | Record<string, unknown>forjson_metadatainstead ofanyfor better type safety, though the current approach may be intentional for flexibility with varying Hive post structures.packages/renderer/package.json (1)
3-3: LGTM on version bump and dependency update.The package version increment to 1.4.4 and the
@ecency/render-helperupdate to ^2.4.4 are consistent with the changes across the monorepo.Also applies to: 47-47
packages/render-helper/src/index.ts (1)
8-8: LGTM on the type export.The type-only import and export pattern is correctly implemented, making
Entryavailable to package consumers without any runtime overhead.Also applies to: 20-21
apps/web/package.json (1)
20-20: LGTM on the dependency update.The
@ecency/render-helperversion is consistent with the update inpackages/renderer/package.json.apps/web/src/app/decks/_components/columns/deck-whats-new-column.tsx (2)
6-6: LGTM on the import update.The switch from
markdownToHTMLtorenderPostBodyaligns with the render-helper package API.
80-83: XSS sanitization is properly implemented via thexsslibrary.The
renderPostBodyfunction applies comprehensive XSS protection through a customsanitizeHtmlmethod (inpackages/render-helper/src/methods/sanitize-html.method.ts) that uses thexsslibrary with:
- Whitelist-based tag and attribute filtering (
ALLOWED_ATTRIBUTES)- Event handler blocking (attributes starting with
onare stripped)- JavaScript URL blocking in
srcattributes- Style attributes entirely disabled
- ID validation against a regex whitelist
Sanitization is applied both during markdown processing (as pre-sanitization for malformed HTML) and as final output processing before returning from
markdownToHTML. The GitHub release markdown is therefore safely protected against XSS injection.packages/renderer/src/lib/api/hive.api.ts (1)
9-24: LGTM!The caching logic and API call are straightforward. The cache key pattern and type assertion are appropriate for this use case.
packages/renderer/src/lib/components/extensions/wave-like-post-extension.tsx (1)
19-51: Component logic looks good overall.The URL parsing, caching via
getCachedPost, and host detection logic are reasonable. Minor observations:
- Line 20:
titlein the state type (Entry & { title: string }) is never used in the render output.- Line 48: The
as anycast loses type safety; this could be avoided if the API returns a properly typed response.These are minor and don't affect functionality.
✏️ Tip: You can disable this entire section by setting review_details to false in your review settings.
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Fix all issues with AI agents
In `@packages/render-helper/src/methods/a.method.ts`:
- Around line 694-697: The assigned embed URL is wrong: replace the legacy watch
URL assigned to videoHref with the proper 3Speak iframe embed endpoint; set
videoHref to "https://play.3speak.tv/embed?v=${match[3]}&mode=iframe" (using the
same match[3]) so the value stored in data-embed-src is valid for iframe
embedding while leaving el.setAttribute('class', ...) and
el.setAttribute('data-embed-src', videoHref) intact.
📜 Review details
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (6)
packages/render-helper/dist/browser/index.jsis excluded by!**/dist/**packages/render-helper/dist/browser/index.js.mapis excluded by!**/dist/**,!**/*.mappackages/render-helper/dist/node/index.cjsis excluded by!**/dist/**packages/render-helper/dist/node/index.cjs.mapis excluded by!**/dist/**,!**/*.mappackages/render-helper/dist/node/index.mjsis excluded by!**/dist/**packages/render-helper/dist/node/index.mjs.mapis excluded by!**/dist/**,!**/*.map
📒 Files selected for processing (3)
packages/render-helper/src/markdown-2-html.spec.tspackages/render-helper/src/methods/a.method.spec.tspackages/render-helper/src/methods/a.method.ts
🔇 Additional comments (2)
packages/render-helper/src/methods/a.method.spec.ts (1)
1047-1060: LGTM!The test expectation correctly aligns with the implementation change in
a.method.ts. The test properly verifies that 3Speak links are processed with the new URL format.packages/render-helper/src/markdown-2-html.spec.ts (1)
420-442: LGTM!The test expectations for 3Speak video handling (tests 26 and 27) are correctly updated to reflect the new embed URL format. Both tests cover different 3Speak TLD variations and properly verify the URL transformation.
✏️ Tip: You can disable this entire section by setting review_details to false in your review settings.
1 participant
Summary by CodeRabbit
Bug Fixes
Tests
Chores
✏️ Tip: You can customize this high-level summary in your review settings.