Render helper migration #615
Conversation
📝 WalkthroughWalkthroughAdds a new package Changes
Sequence Diagram(s)sequenceDiagram
participant Consumer as Consumer
participant API as markdown2Html()
participant Cache as LRU Cache
participant Cleaner as cleanReply()
participant Renderer as markdownToHTML()
participant Traverser as traverse()
participant Handlers as DOM Handlers
participant Sanitizer as sanitizeHtml()
Consumer->>API: render(entry|string, forApp?, webp?)
API->>Cache: cacheGet(key)
alt hit
Cache-->>API: cached HTML
else miss
API->>Cleaner: cleanReply(body)
Cleaner-->>API: cleaned text
API->>Renderer: markdownToHTML(cleaned, forApp, webp)
Renderer->>Traverser: traverse(parsed DOM)
Traverser->>Handlers: process nodes (a/img/iframe/text)
Handlers-->>Traverser: normalized DOM
Traverser-->>Renderer: serialized HTML
Renderer->>Sanitizer: sanitizeHtml(html)
Sanitizer-->>Renderer: sanitized HTML
Renderer-->>API: final HTML
API->>Cache: cacheSet(key, final HTML)
end
API-->>Consumer: HTML
Estimated code review effort🎯 4 (Complex) | ⏱️ ~60 minutes Possibly related PRs
Poem
🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (2 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 (1)
🚧 Files skipped from review as they are similar to previous changes (1)
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: 12
Note
Due to the large number of review comments, Critical, Major severity comments were prioritized as inline comments.
🤖 Fix all issues with AI agents
In @packages/render-helper/package.json:
- Around line 55-67: Remove the unused dependency "react-native-crypto-js" from
the dependencies block in package.json; update the "xmldom" entry to "^0.6.0" to
use the newer stable parser (used by dom-parser.const.ts,
get-inner-html.method.ts, and markdown-to-html.method.ts); and review the pinned
"multihashes" entry (currently "0.4.13") used by proxify-image-src.ts—either
keep the pin if required or update it to a newer compatible release such as
"4.0.3" after running tests and verifying no breaking changes.
In @packages/render-helper/src/catch-post-image.ts:
- Around line 65-68: The function catchPostImage currently casts a string to any
and forwards it to getImage, which causes runtime errors because getImage
expects Entry properties like json_metadata and body; fix by handling string
input explicitly in catchPostImage: when typeof obj === 'string', construct a
minimal Entry-like wrapper (e.g., { body: obj, json_metadata: '{}' } or parsed
metadata if available) and pass that object to getImage(entryWrapper, width,
height, format) instead of casting to any, ensuring getImage receives the
properties it reads.
In @packages/render-helper/src/consts/regexes.const.ts:
- Line 14: The YOUTUBE_REGEX constant currently has the global flag which causes
stateful lastIndex behavior and intermittent failures; either remove the /g flag
from the YOUTUBE_REGEX definition or, if you must keep /g, reset
YOUTUBE_REGEX.lastIndex = 0 immediately before every stateful use (e.g., before
calls to .exec() or .test() in the functions that use it such as those in
a.method (uses around the exec after match) and text.method), ensuring
consistent matching; update the YOUTUBE_REGEX declaration (symbol:
YOUTUBE_REGEX) or add lastIndex resets in the calling sites accordingly.
In @packages/render-helper/src/markdown-2-html.ts:
- Line 12: The cache key built in the markdown2Html implementation omits the
forApp parameter so different forApp values collide; update the key construction
(where const key = `${makeEntryCacheKey(obj)}-md${webp ? '-webp' : ''}` is
defined inside markdown2Html) to include the forApp flag (e.g., append a clear
token/version of forApp such as `-forApp:${forApp}` or `-${forApp ? 'app' :
'site'}`) so cached results are unique per forApp value.
- Line 19: The code mutates the input Entry by assigning back to obj.body;
instead, call cleanReply(obj.body) into a new local variable (e.g., const
cleanBody = cleanReply(obj.body)) and use cleanBody for any further processing
or return a new object with the cleaned value so the original obj is not
modified; update the function that contains the obj/body usage to stop writing
to obj.body and either return {...obj, body: cleanBody} or operate on cleanBody
locally.
In @packages/render-helper/src/methods/linkify.method.ts:
- Around line 41-46: In linkify.method.ts the profile-link branch builds an
invalid attribute when forApp is true by setting attrs to a bare URL; update the
profile-link attrs assignment inside the SECTION_LIST branch (the block that
returns ` <a class="markdown-profile-link" ...>`) to include an explicit
attribute name, e.g. set attrs to `href="https://ecency.com/@${uu}/${permlink}"`
(mirroring the non-app case) so the returned anchor has valid HTML; keep the
rest of the logic (class name and inner text `@${uu}/${permlink}`) unchanged.
In @packages/render-helper/src/methods/remove-child-nodes.method.ts:
- Around line 1-5: The removeChildNodes function fails to remove all children
because it iterates by numeric indices over the live Node.childNodes collection
which shifts as children are removed; fix by iterating over a stable snapshot or
by repeatedly removing the first/last child until none remain (e.g., use
Array.from(node.childNodes) and remove each element, or while (node.firstChild)
removeChild(node.firstChild)); update the implementation in removeChildNodes to
use one of these approaches so no children are skipped.
In @packages/render-helper/src/methods/sanitize-html.method.ts:
- Line 25: The check in sanitize-html.method.ts uses the raw attribute `value`
for the condition if (tag === 'span' && name === 'class' && value === 'wr')
return ''; which can be bypassed with encoded entities; update this check to use
the already-decoded variable `decoded` (and normalize case/whitespace if other
checks do so) so it reads something like checking `decoded === 'wr'` instead of
`value === 'wr'`, ensuring you reference the same decoding/normalization logic
used elsewhere in the function (e.g., the `decoded` variable) to avoid
encoded-entity bypasses.
- Around line 26-28: The ID validation fails because decoded is lowercased
before checking against ID_WHITELIST (regex /^[A-Za-z][-A-Za-z0-9_]*$/), causing
valid IDs with uppercase letters to be rejected; to fix, either perform the
ID_WHITELIST.test(decoded) check before you call toLowerCase() (i.e., validate
the original decoded value when name === 'id') or adjust ID_WHITELIST to only
match lowercase (e.g., change to /^[a-z][-a-z0-9_]*$/) and keep the
lowercasing—update the logic around the name === 'id' branch and the decoded
variable usage accordingly.
In @packages/render-helper/src/methods/text.method.ts:
- Around line 28-59: After replacing the node for images, add an early return to
avoid further processing of the same node; specifically, after the IMG_REGEX
branch where createImageHTML(...) is parsed and
node.parentNode.replaceChild(...) is called, return from the enclosing function
so the subsequent YOUTUBE_REGEX logic (which uses YOUTUBE_REGEX.exec(...),
extractYtStartTime(...), creates thumbImg/play and calls
DOMParser.parseFromString(...) and node.parentNode.replaceChild(...)) cannot run
on an already-removed node. Ensure the same pattern is applied whenever you
replace a node (i.e., exit the function after replacement) to prevent multiple
replacements and errors.
🟡 Minor comments (12)
packages/render-helper/src/methods/iframe.method.ts-33-39 (1)
33-39: Twitch URL parameter concatenation may produce malformed URLs.Line 36 unconditionally prepends
&when appending parameters, but if the originalsrchas no query string, this produces an invalid URL likehttps://player.twitch.tv/...&parent=ecency.com. The URL should use?or&based on whether a query string already exists.Proposed fix
// Twitch if (src.match(TWITCH_EMBED_REGEX)) { const parentDomain = 'ecency.com'; - const s = `${src}&parent=${parentDomain}&autoplay=false`; + const separator = src.includes('?') ? '&' : '?'; + const s = `${src}${separator}parent=${parentDomain}&autoplay=false`; el.setAttribute('src', s); return; }packages/render-helper/src/consts/regexes.const.ts-19-20 (1)
19-20: D_TUBE_REGEX has unescaped metacharacters.Line 19:
d.tube.#!should likely bed\.tube\/#!— the.matches any character (not literal dot), and#within the URL path suggests a hash fragment that needs the/before it escaped as well.Proposed fix
-export const D_TUBE_REGEX = /(https?:\/\/d.tube.#!\/v\/)(\w+)\/(\w+)/g -export const D_TUBE_REGEX2 = /(https?:\/\/d.tube\/v\/)(\w+)\/(\w+)/g +export const D_TUBE_REGEX = /(https?:\/\/d\.tube\/#!\/v\/)(\w+)\/(\w+)/g +export const D_TUBE_REGEX2 = /(https?:\/\/d\.tube\/v\/)(\w+)\/(\w+)/gpackages/render-helper/src/consts/white-list.const.ts-1-18 (1)
1-18: Duplicate entry inWHITE_LIST.
'dapplr.in'appears twice in the list (lines 6 and 14). Remove the duplicate to keep the list clean.Proposed fix
'stemgeeks.net', 'hiveblockexplorer.com', 'proofofbrain.blog', 'weedcash.network', - 'dapplr.in', 'liketu.com', 'bilpcoin.com', 'inji.com'packages/render-helper/src/methods/clean-reply.method.ts-26-26 (1)
26-26: Bug: Case mismatch causes filter to never match.
item.toLowerCase().includes('[via Inbox]')will never be true because a lowercased string cannot contain the uppercase 'I' in'[via Inbox]'. The search string should be all lowercase.Proposed fix
- .filter(item => item.toLowerCase().includes('[via Inbox]') === false) + .filter(item => item.toLowerCase().includes('[via inbox]') === false)packages/render-helper/src/methods/traverse.method.ts-21-23 (1)
21-23: Incorrect type annotation for Text node parameter.The
text()function declares its parameter asHTMLElement | null, but receives Text nodes (#text) which areTextinstances, notHTMLElement. While the current implementation works because both inherit commonNodeproperties (parentNode,nodeValue,ownerDocument), the type annotation is semantically incorrect. Update the function signature intext.method.ts(line 7) to acceptText | Node | nullinstead ofHTMLElement | nullto accurately reflect what it handles.packages/render-helper/src/proxify-image-src.ts-12-18 (1)
12-18: Regex escaping issue in hash extraction.The regex
/.webp/and/.png/don't escape the dot, so they match any character followed bywebp/png(e.g.,xwebp). Additionally, without thegflag, only the first occurrence is replaced.🐛 Proposed fix
export function extractPHash(url: string): string | null { if (url.startsWith(`${proxyBase}/p/`)) { const [hash] = url.split('/p/')[1].split('?') - return hash.replace(/.webp/,'').replace(/.png/,'') + return hash.replace(/\.webp$/i, '').replace(/\.png$/i, '') } return null }packages/render-helper/src/post-body-summary.ts-85-92 (1)
85-92: Decryption runs unconditionally for non-web platforms.The decryption block executes when
platform !== 'web'even ifencEntitiesis empty (no entities were encrypted). This wastes cycles and imports CryptoJS unnecessarily.🐛 Proposed fix
//decrypt and put back entiteis - if(platform !== 'web'){ + if(platform !== 'web' && encEntities.length > 0){ + const CryptoJS = require("react-native-crypto-js"); encEntities.forEach((encEntity)=>{ - var CryptoJS = require("react-native-crypto-js"); let decData = CryptoJS.enc.Base64.parse(encEntity).toString(CryptoJS.enc.Utf8); let entity = CryptoJS.AES.decrypt(decData, 'key').toString(CryptoJS.enc.Utf8); text = text.replace(encEntity, entity); }) }packages/render-helper/src/catch-post-image.ts-71-74 (1)
71-74: Distinguish cache miss from cachednullto avoid redundant recomputation.
cacheGetreturnsundefinedon cache miss, butnullis a valid cached result (meaning "no image found"). The checkif (item)treats bothundefinedandnullas falsy, causing cached "no image" results to be recomputed.🐛 Proposed fix
- const item = cacheGet<string | null>(key) - if (item) { + const item = cacheGet<string | null>(key) + if (item !== undefined) { return item }packages/render-helper/src/methods/a.method.ts-76-80 (1)
76-80: JavaScript href check should be case-insensitive.The current check
href.startsWith('javascript')can be bypassed with mixed-case variants likeJavaScript:orJAVASCRIPT:.🔒 Proposed fix
// Do not allow js hrefs - if (href.startsWith('javascript')) { + if (href.toLowerCase().startsWith('javascript')) { el.removeAttribute('href') return }packages/render-helper/src/methods/a.method.ts-702-708 (1)
702-708: Same null safety issue with img src attribute.Apply the same null-safe pattern here as suggested for the D.Tube handler.
🛡️ Proposed fix
if (imgEls.length === 1) { - const thumbnail = proxifyImageSrc(imgEls[0].getAttribute('src').replace(/\s+/g, ''), 0, 0, webp ? 'webp' : 'match') + const imgSrc = imgEls[0].getAttribute('src') || '' + const thumbnail = proxifyImageSrc(imgSrc.replace(/\s+/g, ''), 0, 0, webp ? 'webp' : 'match') const thumbImg = el.ownerDocument.createElement('img')packages/render-helper/src/methods/a.method.ts-637-646 (1)
637-646: Potential null dereference on missing src attribute.
imgEls[0].getAttribute('src')can returnnullif thesrcattribute is missing, causing.replace()to throw.🛡️ Proposed fix
if (imgEls.length === 1) { - const thumbnail = proxifyImageSrc(imgEls[0].getAttribute('src').replace(/\s+/g, ''), 0, 0, webp ? 'webp' : 'match') + const imgSrc = imgEls[0].getAttribute('src') || '' + const thumbnail = proxifyImageSrc(imgSrc.replace(/\s+/g, ''), 0, 0, webp ? 'webp' : 'match') const thumbImg = el.ownerDocument.createElement('img')packages/render-helper/src/methods/a.method.ts-427-443 (1)
427-443: Potential null dereference when accessing regex exec result.After
BCmatchconfirms a match,BITCHUTE_REGEX.exec(href)is called again. If the regex has global flag, the second exec could return null due to lastIndex state. Access toe[1]on line 430 would then throw.🛡️ Safer approach
const BCmatch = href.match(BITCHUTE_REGEX) if (BCmatch && el.textContent.trim() === href) { - const e = BITCHUTE_REGEX.exec(href) - const vid = e[1] + const vid = BCmatch[1] + if (!vid) return el.setAttribute('class', 'markdown-video-link')
🧹 Nitpick comments (39)
packages/render-helper/.vscode/launch.json (1)
1-18: Consider using${workspaceFolder}instead of${workspaceRoot}.The configuration is functional for debugging Jest tests. One minor improvement:
${workspaceRoot}is deprecated in favor of${workspaceFolder}in modern VSCode.Suggested diff
"runtimeArgs": [ "--inspect-brk", - "${workspaceRoot}/node_modules/.bin/jest", + "${workspaceFolder}/node_modules/.bin/jest", "--runInBand" ],packages/render-helper/LICENSE (1)
1-1: Consider updating the copyright year and entity name.The copyright states "2019 eSteem" but the current project is "Ecency" and the year is 2026. If this code originates from 2019, you may want to update it to something like "2019-2026 Ecency" to reflect the current state and ownership.
packages/render-helper/src/methods/p.method.ts (1)
1-6: Consider a more descriptive function name.The name
pis cryptic and doesn't convey the function's purpose. A name likeensureDirAttributeorsetDefaultDirwould improve readability and discoverability.♻️ Suggested rename
-export function p(el: HTMLElement): void { +export function ensureDirAttribute(el: HTMLElement): void { const dir = el.getAttribute('dir') if (!dir) { el.setAttribute('dir', 'auto') } }packages/render-helper/src/methods/get-inner-html.method.ts (2)
3-11: Function only serializes the first child node, not all children.The function name suggests it returns "inner HTML" but it only serializes
childNodes[0]. If the node has multiple children, only the first will be included in the output. If this is intentional, consider renaming togetSerializedFirstChildfor clarity.If full innerHTML is needed:
♻️ Proposed fix to serialize all children
export function getSerializedInnerHTML(node: Node): string { const XMLSerializer = new xmldom.XMLSerializer() - if (node.childNodes[0]) { - return XMLSerializer.serializeToString(node.childNodes[0]) + let result = '' + for (let i = 0; i < node.childNodes.length; i++) { + result += XMLSerializer.serializeToString(node.childNodes[i]) } - - return '' + return result }
4-4: Consider caching or hoisting the XMLSerializer instance.Creating a new
XMLSerializeron every function call adds unnecessary allocation overhead. SinceXMLSerializeris stateless, it can be instantiated once at module level.♻️ Proposed optimization
import xmldom from 'xmldom' +const serializer = new xmldom.XMLSerializer() + export function getSerializedInnerHTML(node: Node): string { - const XMLSerializer = new xmldom.XMLSerializer() - if (node.childNodes[0]) { - return XMLSerializer.serializeToString(node.childNodes[0]) + return serializer.serializeToString(node.childNodes[0]) } return '' }packages/render-helper/src/cache.ts (1)
11-13: Cache miss returnsundefinedcast toT, which may cause runtime issues.When the key doesn't exist,
cache.get(key)returnsundefined, which gets cast toT. Callers expecting a non-nullableTcould encounter unexpected behavior.Consider returning
T | undefinedto make cache misses explicit:♻️ Proposed fix
-export function cacheGet<T extends unknown>(key: string): T { - return cache.get(key) as T +export function cacheGet<T>(key: string): T | undefined { + return cache.get(key) as T | undefined }.github/PUBLISHING.md (2)
163-169: Wrap bare URLs in markdown link syntax.The bare URLs should use proper markdown link formatting for better readability and compatibility.
📝 Suggested fix
-All packages are published to: https://www.npmjs.com/org/ecency +All packages are published to: <https://www.npmjs.com/org/ecency> View published versions: -- https://www.npmjs.com/package/@ecency/render-helper -- https://www.npmjs.com/package/@ecency/sdk -- https://www.npmjs.com/package/@ecency/wallets -- https://www.npmjs.com/package/@ecency/renderer +- <https://www.npmjs.com/package/@ecency/render-helper> +- <https://www.npmjs.com/package/@ecency/sdk> +- <https://www.npmjs.com/package/@ecency/wallets> +- <https://www.npmjs.com/package/@ecency/renderer>
100-100: Clarify the parallel vs sequential publishing behavior.The statement "The CI workflow handles this automatically by building/publishing in parallel where possible" may be misleading. Looking at the workflow, steps execute sequentially (each build/publish step runs after the previous one completes). The parallelism is implicit in that independent packages can be processed in the same workflow run, but they don't actually run concurrently.
Consider rewording to: "The CI workflow handles this automatically by processing changed packages in the correct order."
packages/render-helper/src/methods/iframe.method.ts (2)
21-23: BITCHUTE handler returns without setting any attributes.Unlike other providers that set
frameborder,sandbox, orallowfullscreen, the BITCHUTE branch does nothing except return early. If this is intentional (preserving the original iframe as-is), consider adding a brief comment to clarify.
141-146: Minor typo: "Brigtheon" should be "Brighteon".Proposed fix
- // Brigtheon + // Brighteon if (src.match(BRIGHTEON_REGEX)) {packages/render-helper/src/helper.spec.ts (1)
3-15: LGTM - basic test coverage for cache key generation.The test correctly validates the concatenation format. Consider adding edge case tests for entries with special characters in
author/permlink(e.g., containing hyphens) or undefined/null fields, as these could produce ambiguous or broken cache keys.packages/render-helper/src/consts/dom-parser.const.ts (1)
4-6: AddfatalErrorhandler to xmldom error configuration.The xmldom DOMParser errorHandler API supports a third callback—
fatalError—which signals non-recoverable parse errors. The current code silenceswarninganderror, but omitsfatalError. Consider whether critical parse failures should be logged or handled explicitly:export const DOMParser = new xmldom.DOMParser({ errorHandler: { warning: noop, error: noop, + fatalError: noop // or throw to stop parsing } })Since this code processes user-generated content (tweets, markdown, images), silencing regular errors may be intentional. However,
fatalErrorindicates non-recoverable problems that merit explicit consideration of desired behavior (log, throw, or continue silently).packages/render-helper/src/consts/section-list.const.ts (1)
1-20: Consider addingas constfor stricter type inference.Adding
as constwould provide literal types instead ofstring[], enabling better type checking when validating section values elsewhere.♻️ Suggested improvement
-export const SECTION_LIST = [ +export const SECTION_LIST = [ 'wallet', 'feed', 'followers', 'following', 'points', 'communities', 'posts', 'blog', 'comments', 'replies', 'settings', 'engine', 'permissions', 'referrals', 'payout', 'activities', 'spk', 'trail' -] +] as constpackages/render-helper/src/post-body-summary.spec.ts (1)
81-136: Tests 8, 9, and 10 share identical names — consider adding descriptive distinctions.All three tests are named "Test with long markdown" but cover different scenarios:
- Test 8: YouTube link + HTML headers + markdown
- Test 9: Nested HTML with images and styled paragraphs
- Test 10: D.tube link + Cloudinary images + markdown headers
Unique names would improve test failure diagnostics.
♻️ Suggested improvement
- it('8- Test with long markdown', () => { + it('8- Test with long markdown containing YouTube URL and HTML headers', () => { ... - it('9- Test with long markdown', () => { + it('9- Test with long markdown containing nested HTML and images', () => { ... - it('10- Test with long markdown', () => { + it('10- Test with long markdown containing Dtube and Cloudinary images', () => {packages/render-helper/src/types/entry.interface.ts (1)
1-7: Consider usingstringinstead ofanyforbody.The
bodyfield is processed as text content throughout the rendering pipeline (markdown-to-html, summary extraction). Usingstringwould provide compile-time safety and prevent accidental misuse.♻️ Suggested improvement
export interface Entry { author?: string permlink?: string last_update?: string - body: any + body: string json_metadata?: any }If there are legitimate cases where
bodymight be non-string, consider a union type likestring | nullor document the expected type.packages/render-helper/src/sanitize-html.spec.ts (1)
3-15: Tests are correct but coverage is limited for a security-critical function.The two tests verify basic functionality. Consider expanding coverage to include:
- Event handler stripping (e.g.,
<img onerror="alert('XSS')">)- Other dangerous protocols (e.g.,
data:,vbscript:)- Nested/malformed tags
- Attribute encoding bypass attempts
packages/render-helper/src/test/data/index.ts (3)
1-1: Inconsistent module syntax: CommonJSrequiremixed with ES module exports.The file uses
require('fs')but exports using ES module syntax. For consistency in a TypeScript project, prefer ES module imports.Suggested fix
-const fs = require('fs') +import fs from 'fs'
3-10: Consider adding error handling and stronger typing.The function reads a file synchronously and parses JSON without error handling. If the file doesn't exist or contains invalid JSON, this will throw an uncaught exception. For test utilities this may be acceptable, but consider:
- Adding a try-catch with a descriptive error message
- Defining a return type interface instead of
any
12-12: Remove unused placeholder export.The
fooconstant appears to be unused test scaffolding. If it's not needed, remove it to avoid confusion.packages/render-helper/src/proxify-image-src.spec.ts (1)
52-75: Test isolation concern:setProxyBasemodifies shared state across tests.Tests 3, 4, and 5 call
setProxyBasewhich modifies global/module-level state. This can cause:
- Test order dependency issues
- Flaky behavior if tests run in parallel
- Pollution affecting other test files
Consider resetting the proxy base in a
beforeEach/afterEachhook or at the end of each test that modifies it.Suggested approach
describe('Proxify image src', () => { afterEach(() => { // Reset to default after each test setProxyBase('https://images.ecency.com') }) // ... existing tests ... })packages/render-helper/src/methods/clean-reply.method.ts (1)
2-27: Performance: RepeatedtoLowerCase()calls on each item for every filter.Each
.filter()call iterates all items and callstoLowerCase()separately. With 24+ filters, this results in 24+toLowerCase()calls per line. Consider a single-pass approach.Optimized approach
const FILTER_PATTERNS = [ 'posted using [partiko', 'posted using [dapplr', // ... other patterns (all lowercase) ]; export function cleanReply(s: string): string { if (!s) return ''; const filtered = s.split('\n') .filter(item => { const lower = item.toLowerCase(); return !FILTER_PATTERNS.some(pattern => lower.includes(pattern)); }) .join('\n'); return filtered .replace('Posted via <a href="https://d.buzz" data-link="promote-link">D.Buzz</a>', '') // ... other replacements }packages/render-helper/src/methods/traverse.method.ts (1)
12-14: Simplify childNodes iteration.The current approach is unnecessarily verbose.
Array.from(node.childNodes)achieves the same result more directly.♻️ Suggested simplification
- Array.from(Array(node.childNodes.length).keys()) - .map(i => node.childNodes[i]) - .forEach(child => { + Array.from(node.childNodes).forEach(child => {packages/render-helper/src/methods/text.method.ts (1)
12-14: Consider case-insensitive comparison for parent node names.The
nodeNameproperty may return uppercase values (e.g.,'A','CODE') depending on the DOM implementation. The current comparison with lowercase strings may fail to match.♻️ Suggested improvement
- if (['a', 'code'].includes(node.parentNode.nodeName)) { + if (['a', 'code'].includes(node.parentNode.nodeName.toLowerCase())) { return }packages/render-helper/src/methods/markdown-to-html.method.ts (3)
77-86: Silent error handling may hide rendering failures.When
md.render()or DOM parsing fails, the function silently returns an empty string without logging. This makes debugging difficult when content fails to render.♻️ Suggested improvement
try { output = md.render(input) const doc = DOMParser.parseFromString(`<body id="root">${output}</body>`, 'text/html') traverse(doc, forApp, 0, webp) output = XMLSerializer.serializeToString(doc) } catch (error) { + console.error('Markdown rendering failed:', error) output = '' }
12-18: Consolidate repeated URL replacement patterns.Multiple similar
replace()calls for leofinance/inleo URLs can be consolidated for maintainability.♻️ Suggested consolidation
- // Internalize leofinance.io links - input = input.replace(new RegExp("https://leofinance.io/threads/view/","g"), "/@"); - input = input.replace(new RegExp("https://leofinance.io/posts/","g"), "/@"); - input = input.replace(new RegExp("https://leofinance.io/threads/","g"), "/@"); - input = input.replace(new RegExp("https://inleo.io/threads/view/","g"), "/@"); - input = input.replace(new RegExp("https://inleo.io/posts/","g"), "/@"); - input = input.replace(new RegExp("https://inleo.io/threads/","g"), "/@"); + // Internalize leofinance.io and inleo.io links + const INTERNAL_LINK_PATTERNS = [ + /https:\/\/(?:leofinance|inleo)\.io\/(?:threads\/view|posts|threads)\//g + ]; + INTERNAL_LINK_PATTERNS.forEach(pattern => { + input = input.replace(pattern, '/@'); + });
6-8: Prefer ES module imports over CommonJS require.Using
require()instead of ES imports creates inconsistency in the codebase and may cause issues with tree-shaking in bundlers.♻️ Suggested improvement
-const lolight = require('lolight') -const { Remarkable } = require('remarkable') -const { linkify } = require('remarkable/linkify') +import lolight from 'lolight' +import { Remarkable } from 'remarkable' +import { linkify } from 'remarkable/linkify'packages/render-helper/src/post-body-summary.ts (3)
42-60: Duplicated Remarkable configuration across files.The Remarkable markdown parser configuration (lines 42-60) is nearly identical to
markdown-to-html.method.ts. Consider extracting this into a shared factory function.♻️ Example shared factory
Create a shared utility, e.g., in
methods/remarkable-factory.ts:import { Remarkable } from 'remarkable' import { linkify } from 'remarkable/linkify' export function createRemarkable(options?: { highlight?: (str: string) => string }): Remarkable { const md = new Remarkable({ html: true, breaks: true, typographer: false, highlight: options?.highlight }).use(linkify) md.core.ruler.enable(['abbr']) md.block.ruler.enable(['footnote', 'deflist']) md.inline.ruler.enable(['footnote_inline', 'ins', 'mark', 'sub', 'sup']) return md }
114-114: Return type should bestringinstead ofany.The function always returns a string (either from cache or computed), but the return type is declared as
any, reducing type safety.♻️ Proposed fix
-export function getPostBodySummary(obj: Entry | string, length?: number, platform?:'ios'|'android'|'web'): any { +export function getPostBodySummary(obj: Entry | string, length?: number, platform?:'ios'|'android'|'web'): string {
67-67: Useconstinstead ofvarfor block-scoped variables.
varis used forCryptoJSdeclarations. Preferconstfor consistency and to avoid hoisting issues.packages/render-helper/src/proxify-image-src.ts (1)
7-10: Use strict equality for string comparison.Line 9 uses
==instead of===for comparing strings. While functionally equivalent here, strict equality is preferred for consistency and to avoid type coercion surprises.♻️ Proposed fix
export function setProxyBase(p: string): void { proxyBase = p - fileExtension = proxyBase == 'https://images.ecency.com'; + fileExtension = proxyBase === 'https://images.ecency.com'; }packages/render-helper/src/index.ts (1)
9-18: Consider exporting types for TypeScript consumers.The
Entrytype from./typesis used bycatchPostImageandpostBodySummarybut isn't exported. TypeScript consumers may need this type to properly type their inputs.♻️ Suggested addition
import { isValidPermlink } from "./helper"; +import type { Entry } from './types' export { renderPostBody, catchPostImage, postBodySummary, proxifyImageSrc, setProxyBase, setCacheSize, SECTION_LIST, - isValidPermlink + isValidPermlink, } + +export type { Entry }packages/render-helper/src/methods/a.method.ts (3)
445-479: Consider using match result directly instead of redundant exec() calls.Rumble and Brighteon handlers call both
match()andexec()on the same regex. While the null checks (if (e[1]),if (e[2])) provide safety, using the match result directly would be more consistent and efficient.
721-734: Consider HTML-escaping values in Twitter blockquote construction.The
urlandauthorvalues are inserted directly into HTML string on line 729. While they originate from regex-matched href content, HTML-escaping would provide defense-in-depth against edge cases.
783-786: Consider addingnoreferrerto external link rel attribute.Adding
noreferreralongsidenoopeneris a security best practice that prevents referrer information leakage to external sites.Suggested enhancement
} else { el.setAttribute('target', '_blank'); - el.setAttribute('rel', 'noopener'); + el.setAttribute('rel', 'noopener noreferrer'); }packages/render-helper/src/markdown-2-html.spec.ts (1)
1-5: Mix of ES imports and CommonJS require statements.Lines 1-2 use ES
importsyntax while lines 4-5 use CommonJSrequire(). Consider using consistent import style throughout.Consistent ES imports
import { getTestData } from './test/data' import { markdown2Html } from './markdown-2-html' -const fs = require('fs') -const path = require('path') +import * as fs from 'fs' +import * as path from 'path'packages/render-helper/src/methods/img.method.ts (1)
51-62: Template literal includes extra whitespace in generated HTML.The multi-line template literal produces HTML with embedded newlines and leading spaces. While browsers collapse whitespace, this could cause minor inconsistencies in test assertions or string comparisons.
Cleaner single-line output
export function createImageHTML(src: string, isLCP: boolean, webp: boolean): string { const loading = isLCP ? 'eager' : 'lazy'; const fetch = isLCP ? 'fetchpriority="high"' : 'decoding="async"'; const proxified = proxifyImageSrc(src, 0, 0, webp ? 'webp' : 'match'); - return `<img - class="markdown-img-link" - src="${proxified}" - loading="${loading}" - ${fetch} - itemprop="image" - />`; + return `<img class="markdown-img-link" src="${proxified}" loading="${loading}" ${fetch} itemprop="image" />`; }packages/render-helper/src/helper.ts (3)
13-15: Cache key generation could produce unexpected results with missing properties.If
entry.author,entry.permlink,entry.last_update, orentry.updatedare undefined, the key will contain literal "undefined" strings, potentially causing cache collisions.Add type safety and validation
-export function makeEntryCacheKey(entry: any): string { - return `${entry.author}-${entry.permlink}-${entry.last_update}-${entry.updated}` +interface CacheableEntry { + author: string; + permlink: string; + last_update?: string; + updated?: string; +} + +export function makeEntryCacheKey(entry: CacheableEntry): string { + return `${entry.author}-${entry.permlink}-${entry.last_update ?? ''}-${entry.updated ?? ''}` }
17-31: Minor typo in comment.Line 23: "famated" should be "formatted".
Fix typo
- return '' + parseInt(t || '0'); //parsing is important as sometimes t is famated '123s'; + return '' + parseInt(t || '0'); //parsing is important as sometimes t is formatted '123s';
61-76: Redundant double-dot check can be removed.Line 73's
!label.includes('..')check is unreachable sincesplit('.')already separates on dots - no individual label can contain... The comment acknowledges this.Remove redundant check
return labels.every(label => { return ( label.length >= 3 && label.length <= 16 && /^[a-z]/.test(label) && // must start with a letter - LABEL_REGEX.test(label) && // a-z0-9, hyphens, no start/end hyphen - !label.includes('..') // double dots are impossible after split, but just in case + LABEL_REGEX.test(label) // a-z0-9, hyphens, no start/end hyphen ); });
📜 Review details
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (8)
packages/render-helper/dist/browser/index.d.tsis excluded by!**/dist/**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/**,!**/*.mappnpm-lock.yamlis excluded by!**/pnpm-lock.yaml
📒 Files selected for processing (67)
.github/PUBLISHING.md.github/workflows/packages.ymlpackage.jsonpackages/render-helper/.editorconfigpackages/render-helper/.gitignorepackages/render-helper/.npmignorepackages/render-helper/.vscode/launch.jsonpackages/render-helper/LICENSEpackages/render-helper/README.mdpackages/render-helper/package.jsonpackages/render-helper/src/cache.tspackages/render-helper/src/catch-post-image.spec.tspackages/render-helper/src/catch-post-image.tspackages/render-helper/src/consts/allowed-attributes.const.tspackages/render-helper/src/consts/dom-parser.const.tspackages/render-helper/src/consts/index.tspackages/render-helper/src/consts/regexes.const.tspackages/render-helper/src/consts/section-list.const.tspackages/render-helper/src/consts/white-list.const.tspackages/render-helper/src/external-types/multihashes.d.tspackages/render-helper/src/helper.spec.tspackages/render-helper/src/helper.tspackages/render-helper/src/index.tspackages/render-helper/src/markdown-2-html.spec.tspackages/render-helper/src/markdown-2-html.tspackages/render-helper/src/methods/a.method.tspackages/render-helper/src/methods/clean-reply.method.tspackages/render-helper/src/methods/get-inner-html.method.tspackages/render-helper/src/methods/iframe.method.tspackages/render-helper/src/methods/img.method.tspackages/render-helper/src/methods/index.tspackages/render-helper/src/methods/linkify.method.tspackages/render-helper/src/methods/markdown-to-html.method.tspackages/render-helper/src/methods/noop.method.tspackages/render-helper/src/methods/p.method.tspackages/render-helper/src/methods/remove-child-nodes.method.tspackages/render-helper/src/methods/sanitize-html.method.tspackages/render-helper/src/methods/text.method.tspackages/render-helper/src/methods/traverse.method.tspackages/render-helper/src/post-body-summary.spec.tspackages/render-helper/src/post-body-summary.tspackages/render-helper/src/proxify-image-src.spec.tspackages/render-helper/src/proxify-image-src.tspackages/render-helper/src/sanitize-html.spec.tspackages/render-helper/src/test/data/index.tspackages/render-helper/src/test/data/json/muratkbesiroglu____sci-fi-novel-underground-city-part-13.jsonpackages/render-helper/src/test/data/json/steemitboard____steemitboard-notify-dunsky-20181210t153450000z.jsonpackages/render-helper/src/test/data/legacy/10.jsonpackages/render-helper/src/test/data/legacy/20.jsonpackages/render-helper/src/test/data/legacy/21.jsonpackages/render-helper/src/test/data/legacy/2112524.jsonpackages/render-helper/src/test/data/legacy/22.jsonpackages/render-helper/src/test/data/legacy/23.jsonpackages/render-helper/src/test/data/legacy/24.jsonpackages/render-helper/src/test/data/legacy/25.jsonpackages/render-helper/src/test/data/legacy/26.jsonpackages/render-helper/src/test/data/legacy/27.JSONpackages/render-helper/src/test/data/legacy/28.jsonpackages/render-helper/src/test/data/legacy/29.jsonpackages/render-helper/src/test/data/legacy/31.jsonpackages/render-helper/src/test/snaps.jsonpackages/render-helper/src/types/entry.interface.tspackages/render-helper/src/types/index.tspackages/render-helper/src/types/xss-white-list.interface.tspackages/render-helper/tsconfig.jsonpackages/render-helper/tsup.config.tspackages/renderer/package.json
🧰 Additional context used
🧬 Code graph analysis (16)
packages/render-helper/src/methods/p.method.ts (3)
packages/render-helper/dist/node/index.cjs (1)
dir(1089-1089)packages/render-helper/dist/browser/index.js (1)
dir(1079-1079)packages/render-helper/dist/node/index.mjs (1)
dir(1079-1079)
packages/render-helper/src/methods/sanitize-html.method.ts (2)
packages/render-helper/src/consts/allowed-attributes.const.ts (1)
ALLOWED_ATTRIBUTES(3-69)packages/render-helper/src/consts/regexes.const.ts (1)
ID_WHITELIST(45-45)
packages/render-helper/src/consts/allowed-attributes.const.ts (1)
packages/render-helper/src/types/xss-white-list.interface.ts (1)
XSSWhiteList(3-7)
packages/render-helper/src/consts/section-list.const.ts (2)
packages/render-helper/src/index.ts (1)
SECTION_LIST(16-16)packages/render-helper/dist/browser/index.d.ts (1)
SECTION_LIST(24-24)
packages/render-helper/src/cache.ts (5)
packages/render-helper/src/index.ts (1)
setCacheSize(15-15)packages/render-helper/dist/browser/index.d.ts (1)
setCacheSize(24-24)packages/render-helper/dist/node/index.cjs (3)
key(1329-1329)key(1390-1390)key(1483-1483)packages/render-helper/dist/browser/index.js (3)
key(1319-1319)key(1380-1380)key(1473-1473)packages/render-helper/dist/node/index.mjs (3)
key(1319-1319)key(1380-1380)key(1473-1473)
packages/render-helper/src/test/data/index.ts (1)
packages/sdk/scripts/validate-dmca-patterns.cjs (1)
fs(17-17)
packages/render-helper/src/helper.spec.ts (1)
packages/render-helper/src/helper.ts (1)
makeEntryCacheKey(13-15)
packages/render-helper/src/methods/linkify.method.ts (4)
packages/render-helper/src/helper.ts (3)
isValidUsername(61-76)sanitizePermlink(32-41)isValidPermlink(43-55)packages/render-helper/src/consts/section-list.const.ts (1)
SECTION_LIST(1-20)packages/render-helper/src/consts/regexes.const.ts (1)
IMG_REGEX(3-3)packages/render-helper/src/methods/img.method.ts (1)
createImageHTML(51-62)
packages/render-helper/src/methods/markdown-to-html.method.ts (3)
packages/render-helper/src/consts/regexes.const.ts (1)
ENTITY_REGEX(43-43)packages/render-helper/src/consts/dom-parser.const.ts (1)
DOMParser(4-6)packages/render-helper/src/methods/traverse.method.ts (1)
traverse(7-33)
packages/render-helper/src/sanitize-html.spec.ts (1)
packages/render-helper/src/methods/sanitize-html.method.ts (1)
sanitizeHtml(9-32)
packages/render-helper/src/catch-post-image.ts (5)
packages/render-helper/src/types/entry.interface.ts (1)
Entry(1-7)packages/render-helper/src/proxify-image-src.ts (1)
proxifyImageSrc(34-77)packages/render-helper/src/markdown-2-html.ts (1)
markdown2Html(6-25)packages/render-helper/src/helper.ts (2)
createDoc(3-11)makeEntryCacheKey(13-15)packages/render-helper/src/cache.ts (2)
cacheGet(11-13)cacheSet(15-17)
packages/render-helper/src/markdown-2-html.spec.ts (2)
packages/render-helper/src/markdown-2-html.ts (1)
markdown2Html(6-25)packages/render-helper/src/test/data/index.ts (1)
getTestData(3-10)
packages/render-helper/src/consts/dom-parser.const.ts (1)
packages/render-helper/src/methods/noop.method.ts (1)
noop(1-1)
packages/render-helper/src/methods/img.method.ts (3)
packages/render-helper/src/index.ts (1)
proxifyImageSrc(13-13)packages/render-helper/src/proxify-image-src.ts (1)
proxifyImageSrc(34-77)packages/render-helper/dist/browser/index.d.ts (1)
proxifyImageSrc(24-24)
packages/render-helper/src/methods/get-inner-html.method.ts (3)
packages/render-helper/dist/node/index.cjs (2)
XMLSerializer(245-245)XMLSerializer(1273-1273)packages/render-helper/dist/browser/index.js (2)
XMLSerializer(235-235)XMLSerializer(1263-1263)packages/render-helper/dist/node/index.mjs (2)
XMLSerializer(235-235)XMLSerializer(1263-1263)
packages/render-helper/src/proxify-image-src.ts (2)
packages/render-helper/src/index.ts (2)
setProxyBase(14-14)proxifyImageSrc(13-13)packages/render-helper/dist/browser/index.js (9)
url(890-890)hash(278-278)e(672-672)e(686-686)realUrl(304-304)pHash(305-305)options(306-309)qs(316-316)b58url(324-324)
🪛 markdownlint-cli2 (0.18.1)
.github/PUBLISHING.md
163-163: Bare URL used
(MD034, no-bare-urls)
166-166: Bare URL used
(MD034, no-bare-urls)
167-167: Bare URL used
(MD034, no-bare-urls)
168-168: Bare URL used
(MD034, no-bare-urls)
169-169: Bare URL used
(MD034, no-bare-urls)
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 Fix all issues with AI agents
In @packages/render-helper/src/methods/text.method.ts:
- Around line 36-41: The code calls YOUTUBE_REGEX.exec(nodeValue) and
immediately accesses e[1], which can throw if exec returns null; update the
logic in the block that defines e (result of YOUTUBE_REGEX.exec(nodeValue)) to
check that e is not null before accessing e[1] (e.g., use if (e && e[1]) or
guard-return), ensuring variables vid, thumbnail (proxifyImageSrc call) and
embedSrc are only computed when e is non-null.
- Around line 50-58: The current code interpolates DOM element objects (thumbImg
and play) into a template string passed to DOMParser.parseFromString, which
yields “[object HTMLElement]”; fix by serializing those elements before
interpolation (e.g., use thumbImg.outerHTML and play.outerHTML or XMLSerializer)
or, better, avoid string parsing entirely: create an anchor via
node.ownerDocument.createElement('a'), set its attributes, append thumbImg and
play as children, wrap in a <p> if needed, then call
node.parentNode.replaceChild(newContainer, node); update references to thumbImg,
play, DOMParser.parseFromString, and replaceChild accordingly.
🧹 Nitpick comments (3)
packages/render-helper/README.md (1)
101-134: Minor markdown formatting issues.Static analysis flagged a few markdown lint issues:
- Line 101: The fenced code block should specify a language (e.g.,
```textor```plaintext)- Lines 132-134: Nested list items use 4-space indentation where 2-space is expected
🔧 Suggested fix
-``` +```text src/ ├── index.ts # Main exportsAnd for the nested list:
- Outputs three bundles: - - `dist/browser/index.js` - Browser ESM with TypeScript declarations - - `dist/node/index.mjs` - Node ESM - - `dist/node/index.cjs` - Node CommonJS + - `dist/browser/index.js` - Browser ESM with TypeScript declarations + - `dist/node/index.mjs` - Node ESM + - `dist/node/index.cjs` - Node CommonJSpackages/render-helper/src/consts/regexes.const.ts (1)
2-4: Global regex flags may cause stateful issues.
IMG_REGEXandIPFS_REGEXuse the global (g) flag. When these regexes are used with.exec()or.test()multiple times, thelastIndexproperty persists between calls, potentially causing unexpected behavior.In
text.method.ts(line 28),IMG_REGEXis used with.match()which is safe, but if these regexes are ever used with.exec()in a loop or across multiple calls, consider resettinglastIndexor creating a new regex instance.packages/render-helper/src/methods/linkify.method.ts (1)
2-2: Unused import.
proxifyImageSrcis imported but never used in this file. Image proxification is handled internally bycreateImageHTML.-import { proxifyImageSrc } from '../proxify-image-src'
📜 Review details
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (7)
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/**,!**/*.mappnpm-lock.yamlis excluded by!**/pnpm-lock.yaml
📒 Files selected for processing (10)
packages/render-helper/LICENSEpackages/render-helper/README.mdpackages/render-helper/package.jsonpackages/render-helper/src/catch-post-image.tspackages/render-helper/src/consts/regexes.const.tspackages/render-helper/src/markdown-2-html.tspackages/render-helper/src/methods/linkify.method.tspackages/render-helper/src/methods/remove-child-nodes.method.tspackages/render-helper/src/methods/sanitize-html.method.tspackages/render-helper/src/methods/text.method.ts
🚧 Files skipped from review as they are similar to previous changes (5)
- packages/render-helper/package.json
- packages/render-helper/LICENSE
- packages/render-helper/src/methods/remove-child-nodes.method.ts
- packages/render-helper/src/catch-post-image.ts
- packages/render-helper/src/methods/sanitize-html.method.ts
🧰 Additional context used
🧬 Code graph analysis (2)
packages/render-helper/src/methods/linkify.method.ts (4)
packages/render-helper/src/helper.ts (3)
isValidUsername(61-76)sanitizePermlink(32-41)isValidPermlink(43-55)packages/render-helper/src/consts/section-list.const.ts (1)
SECTION_LIST(1-20)packages/render-helper/src/consts/regexes.const.ts (1)
IMG_REGEX(3-3)packages/render-helper/src/methods/img.method.ts (1)
createImageHTML(51-62)
packages/render-helper/src/methods/text.method.ts (7)
packages/render-helper/src/methods/linkify.method.ts (1)
linkify(6-61)packages/render-helper/src/consts/dom-parser.const.ts (1)
DOMParser(4-6)packages/render-helper/src/consts/regexes.const.ts (3)
IMG_REGEX(3-3)YOUTUBE_REGEX(14-14)POST_REGEX(5-5)packages/render-helper/src/methods/img.method.ts (1)
createImageHTML(51-62)packages/render-helper/src/proxify-image-src.ts (1)
proxifyImageSrc(34-77)packages/render-helper/src/helper.ts (4)
extractYtStartTime(17-31)sanitizePermlink(32-41)isValidUsername(61-76)isValidPermlink(43-55)packages/render-helper/src/consts/white-list.const.ts (1)
WHITE_LIST(1-18)
🪛 LanguageTool
packages/render-helper/README.md
[uncategorized] ~20-~20: The official name of this software platform is spelled with a capital “H”.
Context: ...norepo with centralized GitHub Actions (.github/workflows/packages.yml) - Automaticall...
(GITHUB)
🪛 markdownlint-cli2 (0.18.1)
packages/render-helper/README.md
101-101: Fenced code blocks should have a language specified
(MD040, fenced-code-language)
132-132: Unordered list indentation
Expected: 2; Actual: 4
(MD007, ul-indent)
133-133: Unordered list indentation
Expected: 2; Actual: 4
(MD007, ul-indent)
134-134: Unordered list indentation
Expected: 2; Actual: 4
(MD007, ul-indent)
🔇 Additional comments (5)
packages/render-helper/src/markdown-2-html.ts (1)
6-25: LGTM!The caching implementation is well-designed:
- String inputs correctly bypass caching (no stable identity)
- Entry objects use a composite cache key that includes content hash for auto-invalidation
- Clean separation between cache lookup and rendering logic
packages/render-helper/src/consts/regexes.const.ts (1)
19-21: D_TUBE_REGEX patterns appear malformed.Line 19's regex
d.tube.#!looks suspicious:
- The dots in
d.tubeshould be escaped asd\.tubeto match literal dots- The
.#!sequence is unusual - the dot before#!matches any character, not a literal dotIs this intentional, or should these be:
-export const D_TUBE_REGEX = /(https?:\/\/d.tube.#!\/v\/)(\w+)\/(\w+)/g -export const D_TUBE_REGEX2 = /(https?:\/\/d.tube\/v\/)(\w+)\/(\w+)/g +export const D_TUBE_REGEX = /(https?:\/\/d\.tube\/#!\/v\/)(\w+)\/(\w+)/g +export const D_TUBE_REGEX2 = /(https?:\/\/d\.tube\/v\/)(\w+)\/(\w+)/gPlease verify the expected DTube URL formats to confirm the correct regex pattern.
packages/render-helper/src/methods/text.method.ts (1)
12-14: Verify nodeName casing with xmldom.The check uses lowercase node names (
'a','code'), butnodeNamebehavior varies:
- Standard browser DOM returns uppercase (
'A','CODE')- xmldom may return lowercase depending on configuration
Since this code uses xmldom's DOMParser, verify that
node.parentNode.nodeNamereturns lowercase as expected, or use case-insensitive comparison:if (['a', 'code'].includes(node.parentNode.nodeName.toLowerCase())) {packages/render-helper/src/methods/linkify.method.ts (2)
41-43: SECTION_LIST matching may produce false positives.The check
SECTION_LIST.some(v => p3.includes(v))will match if any section name appears anywhere in the permlink. For example, a post with permlink"my-wallet-tutorial"would incorrectly be treated as a profile/section link instead of a post link because"wallet"is inSECTION_LIST.Consider using a stricter check that matches the permlink as an exact section or at the start:
if (SECTION_LIST.includes(permlink) || SECTION_LIST.some(v => p3.startsWith(v + '/'))) {Or if the intent is to check whether
p3is a section (not contains one):-if (SECTION_LIST.some(v => p3.includes(v))) { +if (SECTION_LIST.includes(permlink)) {Please verify the intended behavior for section matching.
6-60: Implementation looks solid overall.The linkification logic correctly handles:
- Tag linking with numeric-only tag rejection
- User mentions with validation via
isValidUsername- Internal post/profile links with permlink sanitization and validation
- LCP optimization for the first image
There was a problem hiding this comment.
Actionable comments posted: 3
🤖 Fix all issues with AI agents
In @packages/render-helper/package.json:
- Line 65: Replace the deprecated dependency "xmldom" with the maintained
package "@xmldom/xmldom" in package.json (change the dependency entry "xmldom":
"^0.6.0" to "@xmldom/xmldom" with the desired semver, e.g. "^0.7.0"), then
update any import/require sites in the codebase that reference "xmldom" to
import from "@xmldom/xmldom" instead (search for occurrences of import/require
"xmldom" and update to the new package name), and run install and the test suite
to ensure nothing else breaks.
- Line 62: Add explicit React Native support: update package.json exports to
include a "react-native" entry alongside "node" and "default" (so consumers
resolving for mobile get the react-native build), add a clear note in README
stating React Native (iOS/Android) is supported and explain usage expectations,
and document the platform targeting in code by renaming or annotating the
platform parameter (references: post-body-summary.ts and
markdown-to-html.method.ts where platform !== 'web' triggers
require('react-native-crypto-js')) — either rename parameter to platformTarget
or add JSDoc to platform to make intent explicit for consumers.
In @packages/render-helper/src/methods/sanitize-html.method.ts:
- Around line 4-7: The decodeEntities function uses String.fromCharCode which
fails for Unicode code points > 0xFFFF; update decodeEntities to use
String.fromCodePoint for both decimal and hex branches (convert dec to Number
and parseInt(hex,16) remains) so high-value entities decode correctly; update
the two replace callbacks in decodeEntities to call
String.fromCodePoint(Number(dec)) and String.fromCodePoint(parseInt(hex, 16)).
🧹 Nitpick comments (2)
packages/render-helper/src/methods/sanitize-html.method.ts (2)
20-24: Redundantjavascript:check in URL validation.The
javascript:scheme check is redundant since URLs not starting withhttp://orhttps://are already blocked by the preceding regex test. Both conditions would evaluate to true forjavascript:URLs.Simplified condition
- if (tag === 'img' && name === 'src' && (!/^https?:\/\//.test(decodedLower) || decodedLower.startsWith('javascript:'))) return ''; + if (tag === 'img' && name === 'src' && !/^https?:\/\//.test(decodedLower)) return ''; if ( - tag === 'video' && ['src', 'poster'].includes(name) && - (!/^https?:\/\//.test(decodedLower) || decodedLower.startsWith('javascript:')) + tag === 'video' && ['src', 'poster'].includes(name) && !/^https?:\/\//.test(decodedLower) ) return '';
26-26: Minor: RedundanttoLowerCase().trim()call.
decodedLoweralready contains the lowercased, trimmed value. Use it directly for consistency.Proposed fix
- if (tag === 'span' && name === 'class' && decoded.toLowerCase().trim() === 'wr') return ''; + if (tag === 'span' && name === 'class' && decodedLower === 'wr') return '';
📜 Review details
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (7)
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/**,!**/*.mappnpm-lock.yamlis excluded by!**/pnpm-lock.yaml
📒 Files selected for processing (3)
packages/render-helper/package.jsonpackages/render-helper/src/methods/sanitize-html.method.tspackages/render-helper/src/methods/text.method.ts
🚧 Files skipped from review as they are similar to previous changes (1)
- packages/render-helper/src/methods/text.method.ts
🧰 Additional context used
🧬 Code graph analysis (1)
packages/render-helper/src/methods/sanitize-html.method.ts (2)
packages/render-helper/src/consts/allowed-attributes.const.ts (1)
ALLOWED_ATTRIBUTES(3-69)packages/render-helper/src/consts/regexes.const.ts (1)
ID_WHITELIST(45-45)
🔇 Additional comments (3)
packages/render-helper/src/methods/sanitize-html.method.ts (1)
15-30: No additional validation needed — the xss library's defaultsafeAttrValuealready protectshrefandiframe srcfromjavascript:schemes.When
onTagAttrreturnsundefined(line 30), the xss library automatically applies its built-insafeAttrValuesanitization to all whitelisted attributes. This function is explicitly designed to strip dangerous protocols (includingjavascript:) from URL-like attributes such ashrefandsrc. The code correctly leverages this default behavior rather than reimplementing it.Likely an incorrect or invalid review comment.
packages/render-helper/package.json (2)
1-31: Package structure and exports look well-configured.The dual ESM/CJS setup with proper exports map, browser field for node-fetch/undici shimming, and consistent type declarations provides good compatibility across environments.
Minor note: The
typesfield (line 22) and exports types (line 25) both point to the browser build's.d.ts. Ensure this covers all exports for both browser and node contexts—if they differ, you may need separate type declarations.
59-61: I cannot complete the verification due to a repository access failure. However, my web searches revealed critical discrepancies that prevent me from confirming the review comment's accuracy:
[email protected] does not exist on npm — The multihashes package has versions like v1.0.1 and v4.0.3, but 0.4.13 is not published. This is a fundamental issue with the review comment.
[email protected] — This package does exist and was published September 13, 2015 (10+ years old), supporting the "very old" claim. Note: The code snippet shows
path@^0.12.7(with caret), but the comment references[email protected]without the caret.querystring — Confirmed as deprecated on npm, supporting that recommendation.
Without direct access to the repository's package.json, I cannot verify whether the listed versions are accurate or if there are discrepancies in the review comment itself. The non-existence of [email protected] on npm requires clarification.
There was a problem hiding this comment.
Actionable comments posted: 10
🤖 Fix all issues with AI agents
In @packages/render-helper/src/methods/a.method.ts:
- Around line 428-444: The code calls BITCHUTE_REGEX.exec(href) and immediately
uses e[1] which can be null if the regex has the global flag or exec fails;
replace the second exec with the already-obtained BCmatch (use BCmatch[1] for
the video id) or at minimum guard the exec result (check e is truthy before
accessing e[1]) and/or reset BITCHUTE_REGEX.lastIndex before exec; update the
block around BCmatch, e, and vid to use BCmatch[1] (or a null-check) so vid
cannot be read from a null exec result.
- Around line 76-80: Update the protocol check to be case- and
whitespace-insensitive and include the colon: verify href is truthy, then use
href.trim().toLowerCase().startsWith('javascript:') instead of the current
startsWith('javascript') check, and if true call el.removeAttribute('href') and
return (change the conditional around the existing el.removeAttribute('href')
and return).
In @packages/render-helper/src/methods/text.method.ts:
- Around line 12-14: The early-return check uses node.parentNode.nodeName with a
lowercase array and can fail due to case differences or missing parent; update
the condition in text.method.ts to first ensure node.parentNode exists and
compare node.parentNode.nodeName case-insensitively (e.g., call toLowerCase() on
node.parentNode.nodeName) against the tag list ('a', 'code') so both 'A'/'CODE'
and 'a'/'code' match reliably.
- Around line 82-100: The tag captured from POST_REGEX (postMatch[2]) is
unvalidated and can break attribute boundaries; validate and normalize it before
interpolation by enforcing a strict whitelist/regex (e.g., /^[a-z0-9-]+$/i or
your allowed chars), convert/normalize case if needed, and return early if it
fails; then use the validated/normalized tag when building attrs (same place
where author and permlink are checked in text.method.ts around the
DOMParser/replaceChild logic), so that DOMParser receives only safe attribute
values and prevents attribute-breakout XSS.
In @packages/render-helper/src/post-body-summary.ts:
- Around line 84-88: The catch block around mdd.render(entryBody) currently
swallows errors with console.log; change it to either (a) explicitly handle
failure by setting text = '' and logging the error via a proper logger (e.g.,
processLogger.error) with context about entryBody, or (b) rethrow the caught
error so callers can handle it; update the catch to use one of these behaviors
instead of console.log(err) and ensure the variable text is set or the error is
propagated accordingly.
- Line 129: The function getPostBodySummary currently declares its return type
as any but always returns a string; change the signature to return string
instead of any (i.e., update getPostBodySummary(obj: Entry | string, length?:
number, platform?: 'ios'|'android'|'web'): string) and adjust any related type
annotations or callers that relied on any to accept a string; ensure
imports/types for Entry remain unchanged and run TypeScript checks to catch
needed minor fixes in call sites.
- Around line 134-142: The cache key for summaries omits the platform, causing
collisions; update the key construction in post-body-summary.ts where key is
built (currently `${makeEntryCacheKey(obj)}-sum-${length}`) to include the
platform (e.g., `${makeEntryCacheKey(obj)}-sum-${length}-${platform}` or a
normalized string of platform) so that cacheGet and cacheSet use a
platform-specific key when calling postBodySummary(obj.body, length, platform).
- Around line 69-80: The loop calls require("react-native-crypto-js") inside the
entities.forEach and uses a hardcoded key 'key'; move the import out of the loop
(e.g., import or const CryptoJS = require(...) at top) if you must keep
encryption, but better replace the crypto approach with placeholder
substitution: when matching ENTITY_REGEX produce deterministic unique
placeholders (e.g., __ENTITY_0__, __ENTITY_1__), push the original entity into
encEntities and replace entity with the placeholder in entryBody, and later
reverse the mapping when rendering; if you still need encryption, remove the
hardcoded key and pull a secure key from configuration instead of 'key'.
🧹 Nitpick comments (9)
packages/render-helper/README.md (4)
20-20: Minor: Capitalize "GitHub" consistently.The static analysis tool flagged this, and it's a valid style consistency issue.
📝 Suggested fix
-- Part of vision-next monorepo with centralized GitHub Actions (`.github/workflows/packages.yml`) +- Part of vision-next monorepo with centralized GitHub Actions (`.github/workflows/packages.yml`)The text is correct; ignore this if the linter was triggering on something else in context.
99-125: Add language specifier to the fenced code block.The file structure code block at line 101 is missing a language identifier, which affects syntax highlighting and linting.
📝 Suggested fix
## File Structure -``` +```text src/ ├── index.ts # Main exports
131-136: Fix list indentation to match markdown style.The nested list items use 4-space indentation, but the linter expects 2-space indentation for consistency.
📝 Suggested fix
- Outputs three bundles: - - `dist/browser/index.js` - Browser ESM with TypeScript declarations - - `dist/node/index.mjs` - Node ESM - - `dist/node/index.cjs` - Node CommonJS + - `dist/browser/index.js` - Browser ESM with TypeScript declarations + - `dist/node/index.mjs` - Node ESM + - `dist/node/index.cjs` - Node CommonJS
162-167: Clarify test migration status.Line 164 mentions "Vitest for unit tests (to be migrated from Jest)" - if this migration is complete (as the package.json shows
vitestas the test runner), consider updating this text to avoid confusion.packages/render-helper/package.json (1)
63-63: Add a comment explaining the exact version pin formultihashes.This dependency uses an exact version (
0.4.13) while all other dependencies use caret ranges (^). Newer major versions (3.x and 4.x) are available, indicating breaking changes. To clarify this intentional pin, add a comment in the package.json or README documenting the reason (e.g., compatibility issues with newer versions).packages/render-helper/src/post-body-summary.ts (1)
12-34: Consider extracting magic number10into a named constant.The soft limit buffer
limit + 10on line 25 uses an unexplained magic number. A named constant would clarify intent.Suggested improvement
+const SOFT_LIMIT_BUFFER = 10; + const joint = (arr: string[], limit = 200) => { let result = ''; if (arr) { for (let i = 0; i < arr.length; i++) { // join array with space separator if (result) { result += " "; } // break with length reaches limit if (result.length > limit) { break; } else { // make sure last join doesn't break the limit too much - if ((result + arr[i]).length < limit + 10) { + if ((result + arr[i]).length < limit + SOFT_LIMIT_BUFFER) { result += arr[i]; } else { break; } } } } return result.trim(); };packages/render-helper/src/methods/a.method.ts (1)
781-789: Consider addingnoreferrerto external links.While
rel="noopener"prevents tab-nabbing, addingnoreferrerwould also prevent the referrer header from being sent, improving user privacy.Proposed enhancement
} else { el.setAttribute('target', '_blank'); - el.setAttribute('rel', 'noopener'); + el.setAttribute('rel', 'noopener noreferrer'); }packages/render-helper/src/methods/text.method.ts (1)
46-51: Unusedattrsvariable.The
attrsstring is constructed but never used. The anchor element attributes are correctly set viasetAttributecalls on lines 58-63.Remove dead code
const embedSrc = `https://www.youtube.com/embed/${vid}?autoplay=1` - let attrs = `class="markdown-video-link markdown-video-link-youtube" data-embed-src="${embedSrc}" data-youtube="${vid}"` - //extract start time if available - const startTime = extractYtStartTime(nodeValue); - if(startTime){ - attrs = attrs.concat(` data-start-time="${startTime}"`); - } + const startTime = extractYtStartTime(nodeValue); // Create container paragraphpackages/render-helper/src/methods/markdown-to-html.method.ts (1)
125-133: Same inefficient require pattern in decryption loop.Move the
require("react-native-crypto-js")outside theforEachloop for consistency with the suggested fix in the encryption block.Proposed fix
//decrypt and put back entiteis if(forApp && output){ + const CryptoJS = require("react-native-crypto-js"); encEntities.forEach((encEntity)=>{ - const CryptoJS = require("react-native-crypto-js"); const decData = CryptoJS.enc.Base64.parse(encEntity).toString(CryptoJS.enc.Utf8);
📜 Review details
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (8)
packages/render-helper/dist/browser/index.d.tsis excluded by!**/dist/**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/**,!**/*.mappnpm-lock.yamlis excluded by!**/pnpm-lock.yaml
📒 Files selected for processing (16)
packages/render-helper/README.mdpackages/render-helper/package.jsonpackages/render-helper/src/consts/dom-parser.const.tspackages/render-helper/src/helper.tspackages/render-helper/src/methods/a.method.tspackages/render-helper/src/methods/get-inner-html.method.tspackages/render-helper/src/methods/markdown-to-html.method.tspackages/render-helper/src/methods/sanitize-html.method.tspackages/render-helper/src/methods/text.method.tspackages/render-helper/src/post-body-summary.tspackages/render-helper/src/test/data/legacy/23.jsonpackages/render-helper/src/test/data/legacy/24.jsonpackages/render-helper/src/test/data/legacy/25.jsonpackages/render-helper/src/test/data/legacy/27.JSONpackages/render-helper/src/test/data/legacy/28.jsonpackages/render-helper/vitest.config.ts
🚧 Files skipped from review as they are similar to previous changes (7)
- packages/render-helper/src/test/data/legacy/24.json
- packages/render-helper/src/methods/get-inner-html.method.ts
- packages/render-helper/src/test/data/legacy/27.JSON
- packages/render-helper/src/methods/sanitize-html.method.ts
- packages/render-helper/src/test/data/legacy/25.json
- packages/render-helper/src/test/data/legacy/23.json
- packages/render-helper/src/helper.ts
🧰 Additional context used
🧬 Code graph analysis (1)
packages/render-helper/src/methods/text.method.ts (8)
packages/render-helper/dist/browser/index.js (58)
nodeValue(1142-1142)linkified(1143-1143)doc(183-183)doc(409-409)doc(893-893)doc(1145-1148)doc(1157-1157)doc(1202-1205)doc(1303-1303)doc(1318-1318)doc(1395-1395)isLCP(343-343)isLCP(407-407)isLCP(1155-1155)e(672-672)e(686-686)e(702-702)e(718-718)e(745-745)e(761-761)e(782-782)e(799-799)e(818-818)e(844-844)e(861-861)e(888-888)e(923-923)e(1163-1163)vid(673-673)vid(688-688)vid(704-704)vid(722-722)vid(925-925)vid(1165-1165)attrs(1168-1168)attrs(1201-1201)tag(429-429)tag(494-494)tag(562-562)tag(590-590)tag(608-608)tag(647-647)tag(1196-1196)author(430-430)author(456-456)author(476-476)author(500-500)author(527-527)author(547-547)author(563-563)author(648-648)author(891-891)author(1197-1197)permlink(431-431)permlink(501-501)permlink(564-564)permlink(649-649)permlink(1198-1198)packages/render-helper/src/methods/linkify.method.ts (1)
linkify(6-61)packages/render-helper/src/consts/dom-parser.const.ts (1)
DOMParser(17-21)packages/render-helper/src/consts/regexes.const.ts (3)
IMG_REGEX(3-3)YOUTUBE_REGEX(14-14)POST_REGEX(5-5)packages/render-helper/src/methods/img.method.ts (1)
createImageHTML(51-62)packages/render-helper/src/proxify-image-src.ts (1)
proxifyImageSrc(34-77)packages/render-helper/src/helper.ts (4)
extractYtStartTime(18-32)sanitizePermlink(33-42)isValidUsername(62-77)isValidPermlink(44-56)packages/render-helper/src/consts/white-list.const.ts (1)
WHITE_LIST(1-18)
🪛 LanguageTool
packages/render-helper/README.md
[uncategorized] ~20-~20: The official name of this software platform is spelled with a capital “H”.
Context: ...norepo with centralized GitHub Actions (.github/workflows/packages.yml) - Automaticall...
(GITHUB)
🪛 markdownlint-cli2 (0.18.1)
packages/render-helper/README.md
101-101: Fenced code blocks should have a language specified
(MD040, fenced-code-language)
132-132: Unordered list indentation
Expected: 2; Actual: 4
(MD007, ul-indent)
133-133: Unordered list indentation
Expected: 2; Actual: 4
(MD007, ul-indent)
134-134: Unordered list indentation
Expected: 2; Actual: 4
(MD007, ul-indent)
🔇 Additional comments (12)
packages/render-helper/vitest.config.ts (1)
1-8: LGTM!This is a clean, minimal Vitest configuration appropriate for a Node.js utility package. The
globals: truesetting enables convenient test API usage, andenvironment: 'node'is suitable for server-side HTML/markdown rendering tests.packages/render-helper/src/test/data/legacy/28.json (1)
1-5: LGTM!This test case correctly validates XSS sanitization for
javascript:URLs in TABLE/TD BACKGROUND attributes. The expected result appropriately strips the dangerous attributes while preserving the table structure and auto-closing malformed tags.packages/render-helper/README.md (1)
1-6: Well-documented package overview.The README provides a clear and comprehensive introduction to the package's purpose. The security focus (XSS protection, sanitization, image proxification) is appropriately highlighted upfront.
packages/render-helper/package.json (4)
1-19: Package metadata looks well-configured.The package is correctly set up with:
- Proper repository/homepage/bugs URLs for monorepo structure
"private": falsefor npm publishing"type": "module"for ES module support"files": ["dist"]to only publish build artifacts
23-36: Exports configuration is comprehensive.The conditional exports correctly handle React Native, browser, and Node.js environments with appropriate entry points. The browser shims for
node-fetchandundici(set tofalse) will prevent bundlers from including these Node-specific packages in browser builds.
56-70: Dependencies appear appropriate for the package's functionality.The runtime dependencies align well with the documented features:
xssfor XSS protectionremarkablefor Markdown renderinghtmlparser2+dom-serializerfor HTML parsinglru-cachefor caching@xmldom/xmldomfor DOM operationsreact-native-crypto-jsfor React Native crypto support
64-65: Removepathpolyfill;querystringis actively used and requires verification for replacement.The
pathpolyfill is unused in source code (only appears in tests) and can be safely removed. However,querystringis actively used inproxify-image-src.ts(line 64) to stringify query parameters. WhileURLSearchParamscould be an alternative for browser builds, it would require code changes. Verify whether the package targets both Node.js and browser environments before deciding to replacequerystringwith a modern API alternative.packages/render-helper/src/consts/dom-parser.const.ts (2)
8-15: Error handler design looks appropriate for user-generated content.The lenient approach is sensible for handling malformed HTML from markdown input. Suppressing recoverable errors while logging
fatalErrorin development aids debugging without breaking UX.
17-21: No concurrency concerns with this singleton DOMParser implementation.All usages of
DOMParser.parseFromStringin the codebase are synchronous—no async/await or Promise patterns are present. ThecreateDoc,text, and other functions that callparseFromStringare all synchronous and return immediately. Each call toparseFromStringreturns an independentDocumentinstance without maintaining mutable state across calls, making the singleton pattern safe.packages/render-helper/src/methods/a.method.ts (1)
483-518: YouTube embedding logic looks solid.Proper null checks on
e && e[1], thumbnail proxification, and start time extraction are handled correctly. The data attributes fordata-embed-src,data-youtube, anddata-start-timeare set appropriately.packages/render-helper/src/methods/markdown-to-html.method.ts (2)
84-123: Graceful degradation strategy is well-designed.The three-tier fallback approach (normal parse → htmlparser2 repair → HTML-escape) maintains user experience while protecting against XSS. Using
he.encodein the final fallback ensures content visibility without security risks.
135-140: Final sanitization ensures safe output.The cleanup of
xmlnsattributes and body wrapper tags followed bysanitizeHtmlis a solid approach for producing clean, safe HTML output.
There was a problem hiding this comment.
Actionable comments posted: 12
🤖 Fix all issues with AI agents
In @packages/render-helper/src/methods/a.method.ts:
- Around line 702-710: The code calls imgEls[0].getAttribute('src').replace(...)
which can throw if getAttribute returns null; add a null check and only call
replace/proxifyImageSrc when src is non-null (or fallback to an empty string)
before creating the thumbnail. Update the block around the imgEls handling
(references: imgEls, proxifyImageSrc, getAttribute('src'), thumbImg) to guard
the src value, skip thumbnail creation or use '' as a safe fallback if src is
missing, and only append/remove elements when a valid thumbnail URL was
produced.
- Around line 549-555: The conditional building embedSrc uses e[1] incorrectly
for video embeds (e[1] can be the literal "videos"); change the logic in the
embedSrc assignment so that if e[1] is undefined you build the channel URL, if
e[1] === "videos" you use e[2] as the video ID, otherwise use e[1] as the video
ID; update the branch that sets embedSrc (variable embedSrc in this
file/function) to reference e[2] when e[1] === "videos" so the resulting Twitch
URL uses the actual video ID.
- Around line 637-649: The code calls imgEls[0].getAttribute('src').replace(...)
which can throw if getAttribute returns null; update the block around imgEls and
proxifyImageSrc so you first read const src = imgEls[0].getAttribute('src'); if
src is falsy then skip creating the thumbnail (or use an empty string/default)
and avoid calling .replace or passing null into proxifyImageSrc; ensure thumbImg
creation/appending and el.removeChild(imgEls[0]) only occur when src is a
non-empty string so proxifyImageSrc receives a valid string.
- Around line 569-591: The regexes SPOTIFY_REGEX, D_TUBE_REGEX, D_TUBE_REGEX2,
and TWITTER_REGEX are declared with the global flag which makes using match()
then exec() on the same regex instance unreliable due to lastIndex state; fix by
either removing the global flag from those regex definitions or by resetting
each regex's lastIndex to 0 before calling exec() (e.g., set
SPOTIFY_REGEX.lastIndex = 0 before SPOTIFY_REGEX.exec(href)), and apply the same
change for D_TUBE_REGEX, D_TUBE_REGEX2, and TWITTER_REGEX where match()/exec()
pairs are used.
In @packages/render-helper/src/methods/markdown-to-html.method.ts:
- Around line 120-126: The loop that restores placeholders in
markdown-to-html.method.ts (inside the forApp block using
entityPlaceholders.forEach) uses output.replace which only replaces the first
occurrence; update the replacement to replace all occurrences of each
placeholder (e.g., use output = output.split(placeholder).join(entity) or output
= output.replaceAll(placeholder, entity)) so every instance of
`__ENTITY_${index}__` in `output` is restored.
- Around line 62-69: The current loop uses input.replace(entity, placeholder)
which only replaces the first occurrence and, combined with duplicate entries in
entities, breaks restoration; fix by deduplicating entities before creating
placeholders (e.g., build a uniqueEntities array or Set from entities) and then
replace all occurrences when injecting placeholders using input =
input.replaceAll(entity, placeholder) or input = input.replace(new
RegExp(escapeRegExp(entity), 'g'), placeholder); also ensure entityPlaceholders
stores the mapping between placeholder and the original entity (placeholder ->
entity) so later restoration uses the correct value.
In @packages/render-helper/src/methods/text.method.ts:
- Line 85: The regex used in the replacement inside the whitelist check is
unescaped and matches any character after "www" — update the replacement in the
expression `postMatch[1].replace(/www./,'')` to use an escaped dot and anchor to
the start so it only strips a leading "www.": use
`postMatch[1].replace(/^www\./, '')` (update the call around the
`WHITE_LIST.includes(...)` check in the same block).
- Around line 47-52: Remove the dead `attrs` build: the `attrs` variable
declaration and its `concat` usage (the `let attrs = ...` line, the `//extract
start time...` comment and the `if(startTime){ attrs = attrs.concat(...) }`
block) are unused because attributes are set directly later (see the subsequent
attribute-setting block around lines 59-64); delete these lines to avoid the
unused variable and leave only the working programmatic attribute assignment
that uses `nodeValue`, `embedSrc`, `vid`, and `extractYtStartTime`.
- Around line 20-28: The parsed HTML currently grabs doc.documentElement (the
<html> element) which is wrong; change the DOM extraction to use
doc.body.firstChild (or doc.body.firstElementChild) so you insert the parsed
<span> (or image/post nodes) rather than the whole document; if parseFromString
can produce multiple sibling nodes, iterate doc.body.childNodes and insert each
before the original node, then remove the original node. Apply this fix to the
DOMParser.parseFromString usage that sets replaceNode (the block using
DOMParser.parseFromString and node.parentNode.insertBefore/removeChild) and
likewise update the two other occurrences you noted for the image path and post
reference handling.
In @packages/render-helper/src/post-body-summary.ts:
- Around line 36-42: The JSDoc for the function that generates a text summary
(parameters entryBody, length, platform) is misleading: it references "crypto
implementation" and "react-native-crypto-js" even though no crypto is used;
update the comment for the function in post-body-summary.ts to remove any crypto
references and clearly state that the platform parameter controls how
HTML/entities/placeholders are handled for 'web' vs 'ios'/'android', and adjust
the @param descriptions (entryBody, length default 200, platform default 'web')
to reflect actual behavior.
- Around line 69-79: The current loop uses entryBody.replace(entity,
placeholder) which only replaces the first occurrence of an entity; update the
replacement to replace all occurrences (e.g., entryBody =
entryBody.replaceAll(entity, placeholder) or entryBody = entryBody.replace(new
RegExp(escapeRegExp(entity), 'g'), placeholder) if you need
regex-escaping/compatibility) so every instance of the matched entity is
replaced; keep ENTITY_REGEX, entityPlaceholders, and the placeholder naming
(e.g., __ENTITY_${index}__) as-is so restoration logic still maps placeholders
to entityPlaceholders.
🧹 Nitpick comments (3)
packages/render-helper/src/methods/a.method.ts (1)
88-97: Add null check before replaceChild.If
DOMParser.parseFromStringfails or returns an unexpected structure,replaceNodecould benull, causingreplaceChildto throw. Given the lenient error handler in DOMParser, this is unlikely but worth guarding.Suggested defensive check
const doc = DOMParser.parseFromString(imgHTML, 'text/html'); const replaceNode = doc.documentElement || doc.firstChild + if (!replaceNode) { + return + } + el.parentNode.replaceChild(replaceNode, el)packages/render-helper/src/methods/markdown-to-html.method.ts (2)
6-10: Consider using ES module imports for consistency.Mixing ES6 imports with CommonJS
require()works but reduces consistency. If these packages support ES modules, consider usingimportsyntax throughout.
82-118: Error recovery strategy is well-designed.The tiered approach (sanitize → repair with htmlparser2 → re-parse) is robust. The final fallback with HTML escaping ensures content visibility while preventing XSS.
Minor note: the
require()calls inside the catch block incur a small overhead on each recovery. Consider hoisting these imports to module level if recovery is expected to be common.
📜 Review details
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (7)
packages/render-helper/dist/browser/index.d.tsis excluded by!**/dist/**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 (6)
packages/render-helper/src/methods/a.method.tspackages/render-helper/src/methods/markdown-to-html.method.tspackages/render-helper/src/methods/text.method.tspackages/render-helper/src/post-body-summary.tspackages/render-helper/src/test/data/legacy/22.jsonpackages/render-helper/src/test/data/legacy/23.json
🚧 Files skipped from review as they are similar to previous changes (1)
- packages/render-helper/src/test/data/legacy/23.json
🧰 Additional context used
🧬 Code graph analysis (3)
packages/render-helper/src/methods/a.method.ts (9)
packages/render-helper/src/consts/regexes.const.ts (23)
IMG_REGEX(3-3)IPFS_REGEX(4-4)POST_REGEX(5-5)MENTION_REGEX(7-7)INTERNAL_POST_TAG_REGEX(11-11)INTERNAL_MENTION_REGEX(9-9)TOPIC_REGEX(8-8)INTERNAL_TOPIC_REGEX(10-10)CUSTOM_COMMUNITY_REGEX(13-13)CCC_REGEX(6-6)BITCHUTE_REGEX(18-18)RUMBLE_REGEX(33-33)BRIGHTEON_REGEX(34-34)YOUTUBE_REGEX(14-14)VIMEO_REGEX(16-16)TWITCH_REGEX(22-22)SPOTIFY_REGEX(32-32)LOOM_REGEX(40-40)D_TUBE_REGEX(19-19)D_TUBE_REGEX2(20-20)SPEAK_REGEX(29-29)TWITTER_REGEX(31-31)SECTION_REGEX(44-44)packages/render-helper/src/methods/get-inner-html.method.ts (1)
getSerializedInnerHTML(3-11)packages/render-helper/src/methods/img.method.ts (2)
createImageHTML(51-62)img(3-49)packages/render-helper/src/consts/dom-parser.const.ts (1)
DOMParser(17-21)packages/render-helper/src/methods/remove-child-nodes.method.ts (1)
removeChildNodes(1-6)packages/render-helper/src/consts/white-list.const.ts (1)
WHITE_LIST(1-18)packages/render-helper/src/helper.ts (3)
sanitizePermlink(33-42)isValidPermlink(44-56)extractYtStartTime(18-32)packages/render-helper/src/consts/section-list.const.ts (1)
SECTION_LIST(1-20)packages/render-helper/src/proxify-image-src.ts (1)
proxifyImageSrc(34-77)
packages/render-helper/src/post-body-summary.ts (4)
packages/render-helper/src/consts/regexes.const.ts (1)
ENTITY_REGEX(43-43)packages/render-helper/src/types/entry.interface.ts (1)
Entry(1-7)packages/render-helper/src/helper.ts (1)
makeEntryCacheKey(14-16)packages/render-helper/src/cache.ts (2)
cacheGet(11-13)cacheSet(15-17)
packages/render-helper/src/methods/markdown-to-html.method.ts (4)
packages/render-helper/src/consts/regexes.const.ts (1)
ENTITY_REGEX(43-43)packages/render-helper/src/consts/dom-parser.const.ts (1)
DOMParser(17-21)packages/render-helper/src/methods/traverse.method.ts (1)
traverse(7-33)packages/render-helper/src/methods/sanitize-html.method.ts (1)
sanitizeHtml(9-33)
🔇 Additional comments (14)
packages/render-helper/src/test/data/legacy/22.json (1)
1-5: LGTM! Good XSS sanitization test coverage.This test fixture exercises critical XSS vectors including case-varied
javascript:protocols, HTML-entity obfuscation, event handler injections, and malformed tag attacks. The expected output correctly shows these vectors being neutralized through HTML-escaping, attribute stripping, and script tag removal.packages/render-helper/src/methods/text.method.ts (1)
90-94: Good XSS prevention.The validation checks for tag, author, and permlink properly prevent attribute breakout attacks. The defensive early returns are well-placed.
packages/render-helper/src/methods/a.method.ts (5)
1-34: LGTM!Imports are well-organized and appropriately scoped. The module pulls in necessary regex constants, DOM utilities, and helper functions.
36-55: LGTM!Helper functions are concise with proper null handling. The
getInlineMetautility cleanly encapsulates the logic for determining inline vs. non-inline link display.
58-80: LGTM!Good defensive checks: null element/parent handling, early return on missing href, preserving existing author/tag links, and blocking
javascript:URLs with case-insensitive check.
179-241: LGTM!The tagged post and profile section link handling correctly validates permlinks and sections, with appropriate whitelist checks and proper handling of both app and web contexts.
755-788: LGTM!External link handling is solid: proper scheme normalization,
target="_blank"withrel="noopener"for security, and appropriate data attributes for app context.packages/render-helper/src/methods/markdown-to-html.method.ts (2)
12-18: LGTM!URL internalization for leofinance.io and inleo.io threads is straightforward. The replacements correctly convert external thread URLs to internal
/@format.
128-134: LGTM!Output cleanup properly removes xmldom artifacts and applies final sanitization. The double-sanitization in the recovery path adds defense-in-depth.
packages/render-helper/src/post-body-summary.ts (5)
1-11: LGTM!Imports are appropriate. Using
hefor HTML entity decoding is the right choice for this text extraction use case.
12-34: LGTM!The
joint()helper implements sensible word-based truncation with a small buffer to avoid cutting words mid-way. The null check on line 14 provides safety.
83-94: LGTM!Error handling logs useful debugging context without exposing the full body content. Empty string fallback is safe.
106-122: LGTM!Text processing pipeline is well-ordered: strip HTML → remove URLs → collapse whitespace → truncate → decode entities. The
he.decode()at the end ensures clean output.
133-149: LGTM!The public API correctly handles both string and Entry inputs. Cache key construction properly incorporates all relevant parameters (entry key, length, platform) to avoid cache collisions.
There was a problem hiding this comment.
Actionable comments posted: 8
🤖 Fix all issues with AI agents
In @packages/render-helper/src/methods/a.method.ts:
- Around line 766-782: The href handling can throw on decodeURI and the second
condition ignores scheme-less links; wrap the decodeURI(href).match(...) call in
a try/catch and bail out on decode errors, and change the indexOf check from > 0
to >= 0 (or use
href.indexOf('hivesigner.com/sign/update-proposal-votes?proposal_ids') !== -1 /
href.includes(...)) so links without "https://" are matched; keep the existing
DOM updates (el.setAttribute, el.removeAttribute) and return only when decode
succeeds and the regex match is found.
- Around line 82-97: The current logic parses imgHTML with DOMParser and then
uses doc.documentElement (the <html> of a different Document) and passes that
foreign node to el.parentNode.replaceChild, which can throw or insert an entire
document; instead, create a node belonging to the same document as el (use
el.ownerDocument) and import/parse safely: either import doc.body.firstChild via
el.ownerDocument.importNode(doc.body.firstChild, true) and replace el with that
imported node, or parse imgHTML into a DocumentFragment in el.ownerDocument via
el.ownerDocument.createRange().createContextualFragment(imgHTML) and replace el
with fragment.firstChild; update the code paths that call
DOMParser.parseFromString, replaceChild, and replace doc.documentElement usage
(refer to IMG_REGEX, getSerializedInnerHTML, createImageHTML,
DOMParser.parseFromString, el.parentNode.replaceChild) accordingly.
- Around line 36-55: getInlineMeta currently returns nonInline: textMatches ||
titleMatches which inverts the intended meaning; change the return to clearly
represent inline status (e.g., return { textMatches, contentMatches: textMatches
|| titleMatches } or better return { textMatches, isInline: textMatches ||
titleMatches }) and update any callers that read the old nonInline property to
use the new property (or invert their logic if you prefer renaming). Ensure
matchesHref and normalizeValue remain unchanged and only adjust the property
name/boolean so that links with matching text/title yield isInline=true and bare
URLs yield isInline=false.
- Around line 121-426: The whitelist/domain checks are unsafe because
replace(/www./,'') uses a wildcard dot and includes() is used against captured
strings that contain paths/subdomains; update all occurrences (e.g., where
POST_REGEX, MENTION_REGEX, INTERNAL_POST_TAG_REGEX, INTERNAL_MENTION_REGEX,
INTERNAL_POST_REGEX, TOPIC_REGEX, CUSTOM_COMMUNITY_REGEX, CCC_REGEX are used) to
(1) normalize the captured host by extracting only the host/domain portion
(strip protocol, path and port) and (2) replace the unsafe replace(/www./,'')
with .replace(/^www\./i,'') or better yet test the extracted domain, and (3)
change any WHITE_LIST.some(v => domain.includes(v)) patterns to exact checks
like WHITE_LIST.includes(domain) (or compare against canonicalized domain)
before trusting the link; apply these fixes wherever domain validation occurs
(e.g., postMatch[1], mentionMatch[1], tpostMatch[1], cpostMatch[1],
topicMatch[1], comMatch[1], cccMatch[1]) so only literal whitelist domains pass.
In @packages/render-helper/src/post-body-summary.ts:
- Around line 69-83: The placeholder tokens like "__ENTITY_${index}__" are being
parsed by Markdown and lost; change the placeholder format to a markdown-inert
token (e.g., wrap the index in private-use Unicode markers such as U+E000/U+E001
or another sequence guaranteed not to be interpreted by Markdown) when you
generate placeholders in the block using ENTITY_REGEX and entityPlaceholders,
replace occurrences in entryBody with that inert placeholder, push the original
entity into entityPlaceholders as before, and ensure the restoration logic later
searches for and replaces that exact inert token back to the original entity
(apply the same change to the analogous code around the 100-107 region).
- Around line 43-47: postBodySummary currently only truncates when length is
truthy and treats undefined/omitted differently from the docs; change the
parameter handling to default undefined to 200 and then apply truncation only
when length > 0 (treat length === 0 as “no truncation”); implement by replacing
the truthy check with something like: length = length === undefined ? 200 :
length; if (length > 0) { /* truncate */ } and apply the same fix to the other
two truncation sites in this file that use the length parameter so behavior
matches the docs.
- Around line 137-153: getPostBodySummary passes a possibly undefined platform
(and length) into postBodySummary which expects a non-undefined platform,
causing strictNullChecks type errors and an inconsistent cache key; normalize
inputs at the top of getPostBodySummary (e.g., set const normalizedPlatform =
platform ?? 'web' and const normalizedLength = length ?? <default>) and use
those normalized values when building key (`makeEntryCacheKey(...)`), calling
`postBodySummary(obj.body, normalizedLength, normalizedPlatform)` and when
calling `postBodySummary` for string input as well, ensuring
`cacheGet`/`cacheSet` use the same normalized values.
- Around line 1-11: Replace the CommonJS requires with ESM imports: import
Remarkable and its linkify plugin using TypeScript/ESM syntax instead of
require() in post-body-summary.ts (look for the Remarkable and linkify usages),
ensuring you import types from @types/remarkable if needed and preserve any
plugin initialization; apply the same change in
packages/render-helper/src/methods/markdown-to-html.method.ts where Remarkable
and linkify are required.
🧹 Nitpick comments (7)
packages/render-helper/src/methods/markdown-to-html.method.ts (3)
24-30: Consider precompiling regexes with proper escaping.The
.character in URLs likeleofinance.iois not escaped, so it matches any character (e.g.,leofinanceXiowould also match). Additionally, creating newRegExpobjects on every call is inefficient.♻️ Suggested refactor: Use precompiled regex literals
+// Precompiled regexes for URL normalization +const LEOFINANCE_THREADS_VIEW_REGEX = /https:\/\/leofinance\.io\/threads\/view\//g +const LEOFINANCE_POSTS_REGEX = /https:\/\/leofinance\.io\/posts\//g +const LEOFINANCE_THREADS_REGEX = /https:\/\/leofinance\.io\/threads\//g +const INLEO_THREADS_VIEW_REGEX = /https:\/\/inleo\.io\/threads\/view\//g +const INLEO_POSTS_REGEX = /https:\/\/inleo\.io\/posts\//g +const INLEO_THREADS_REGEX = /https:\/\/inleo\.io\/threads\//g + export function markdownToHTML(input: string, forApp: boolean, webp: boolean): string { // Internalize leofinance.io links - input = input.replace(new RegExp("https://leofinance.io/threads/view/","g"), "/@"); - input = input.replace(new RegExp("https://leofinance.io/posts/","g"), "/@"); - input = input.replace(new RegExp("https://leofinance.io/threads/","g"), "/@"); - input = input.replace(new RegExp("https://inleo.io/threads/view/","g"), "/@"); - input = input.replace(new RegExp("https://inleo.io/posts/","g"), "/@"); - input = input.replace(new RegExp("https://inleo.io/threads/","g"), "/@"); + input = input.replace(LEOFINANCE_THREADS_VIEW_REGEX, "/@"); + input = input.replace(LEOFINANCE_POSTS_REGEX, "/@"); + input = input.replace(LEOFINANCE_THREADS_REGEX, "/@"); + input = input.replace(INLEO_THREADS_VIEW_REGEX, "/@"); + input = input.replace(INLEO_POSTS_REGEX, "/@"); + input = input.replace(INLEO_THREADS_REGEX, "/@");
70-72: Consider moving the early return before Remarkable initialization.The empty input check happens after the Remarkable parser is fully configured. Moving it earlier (e.g., right after line 30) would avoid unnecessary setup work for empty inputs.
♻️ Suggested change
input = input.replace(new RegExp("https://inleo.io/threads/","g"), "/@"); + if (!input) { + return '' + } const md = new Remarkable({ // ... configuration }) // ... rest of setup const serializer = new XMLSerializer() - if (!input) { - return '' - }
114-115: Hoist dynamicrequire()calls to module level.The
htmlparser2,dom-serializer, andhelibraries are dynamically required inside catch blocks. This means on every parsing error, Node.js must resolve and potentially load these modules, which is expensive. Hoisting them to the top of the file ensures they're loaded once at module initialization.♻️ Suggested refactor
Add to the imports section at the top of the file:
import { traverse } from './traverse.method' import { sanitizeHtml } from './sanitize-html.method' import { DOMParser, ENTITY_REGEX } from '../consts' import { XMLSerializer } from '@xmldom/xmldom' +import * as htmlparser2 from 'htmlparser2' +import domSerializer from 'dom-serializer' +import he from 'he' const { Remarkable } = require('remarkable') const { linkify } = require('remarkable/linkify')Then update the catch blocks:
- const htmlparser2 = require('htmlparser2') - const domSerializer = require('dom-serializer').default const dom = htmlparser2.parseDocument(preSanitized, {- const he = require('he') const escapedContent = he.encode(output || md.render(input))Also applies to: 135-135
packages/render-helper/src/post-body-summary.ts (1)
43-68: Consider reusing a singleRemarkableinstance (hot-path perf).
new Remarkable(...).use(...)and all the ruler enables happen on every call; if summaries are generated frequently, this will be a noticeable cost.packages/render-helper/src/methods/a.method.ts (3)
58-80: Goodjavascript:stripping; consider also blockingdata:URLs if not already sanitized upstream.
735-764: Twitter embed DOM construction is the right direction; avoid confusing shadowing (const a = ...) insidefunction a().
784-817: External link fallback: considerrel="noopener noreferrer"and avoid overwriting existing classes.Minor hardening: add
noreferrerfor_blanklinks (if acceptable) and preferclassList.add(...)so earlier styling hooks aren’t lost when settingclassrepeatedly.
📜 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 (4)
packages/render-helper/src/methods/a.method.tspackages/render-helper/src/methods/markdown-to-html.method.tspackages/render-helper/src/methods/text.method.tspackages/render-helper/src/post-body-summary.ts
🚧 Files skipped from review as they are similar to previous changes (1)
- packages/render-helper/src/methods/text.method.ts
🧰 Additional context used
🧬 Code graph analysis (1)
packages/render-helper/src/methods/markdown-to-html.method.ts (4)
packages/render-helper/src/consts/regexes.const.ts (1)
ENTITY_REGEX(43-43)packages/render-helper/src/consts/dom-parser.const.ts (1)
DOMParser(17-21)packages/render-helper/src/methods/traverse.method.ts (1)
traverse(7-33)packages/render-helper/src/methods/sanitize-html.method.ts (1)
sanitizeHtml(9-33)
🔇 Additional comments (4)
packages/render-helper/src/methods/markdown-to-html.method.ts (3)
1-21: LGTM!The lazy-loading pattern for
lolightis a good approach for handling optional dependencies that may not be available in browser builds. The fallback to returning the original string ensures graceful degradation.
33-67: LGTM!The Remarkable configuration is well-structured with appropriate fallback in the syntax highlighter. Enabling
html: trueis acceptable since the output goes throughsanitizeHtmlat the end.
141-155: LGTM!The entity restoration correctly reverses the placeholder substitution using the same index-based mapping. The final cleanup and sanitization ensure the output is well-formed and safe.
packages/render-helper/src/methods/a.method.ts (1)
428-733: lastIndex resets are already in place; Twitch parent param is added in post-processing.The regex
exec()concern is overstated. Only regexes with/gor/yflags requirelastIndexresets:SPOTIFY_REGEX,D_TUBE_REGEX,D_TUBE_REGEX2, andTWITTER_REGEXall have these resets properly applied. The other regexes (RUMBLE,BRIGHTEON,YOUTUBE,VIMEO,TWITCH,LOOM,SPEAK) use only/iflag and do not need resets.Twitch embeds are not missing the
parent=parameter—it is added by the post-processingiframe.method.ts(line 36), which normalizes all iframe sources after initial creation.YouTube, Vimeo, and Twitch embeds in lines 428–733 set
data-embed-srcattributes rather than creating iframes directly; actual iframe elements are created and processed separately. Only Spotify and Loom iframes setsandboxattributes at this stage. Vimeo and Twitch do lack explicitallowpolicies andloadingattributes in their current iframe creation.Likely an incorrect or invalid review comment.
There was a problem hiding this comment.
Actionable comments posted: 7
🤖 Fix all issues with AI agents
In @apps/web/src/features/wallet/operations/wallet-operations-sign.tsx:
- Around line 68-75: The nested ternary that computes keychainAuthority from
authority incorrectly falls back to "Memo" for any unrecognized value; instead
validate authority explicitly (check that authority equals "active", "posting",
or "owner") and handle invalid values by throwing or returning an error before
mapping to "Active"/"Posting"/"Owner"/"Memo"; update the logic around the
authority variable and the keychainAuthority assignment in
wallet-operations-sign (replace the ternary) to perform explicit validation and
fail fast on unknown strings to avoid silently treating typos as "Memo".
In @packages/render-helper/src/methods/a.method.ts:
- Around line 804-820: The code uses YOUTUBE_REGEX.exec(href) after
href.match(YOUTUBE_REGEX), which can misbehave due to RegExp.lastIndex state;
replace the second exec call by using the already-obtained match (use match[1])
or reset YOUTUBE_REGEX.lastIndex = 0 before exec, so inside the forApp block use
the match array returned by href.match(YOUTUBE_REGEX) to extract the video id
and avoid calling YOUTUBE_REGEX.exec(href) (also ensure extractYtStartTime(href)
is unchanged).
- Around line 490-525: The YOUTUBE_REGEX is stateful (likely has the /g flag)
and its lastIndex must be reset before calling exec here (and at the other
occurrence) to avoid false negatives; update this block to either use the
already-obtained match groups (use the result from href.match(YOUTUBE_REGEX)
instead of calling YOUTUBE_REGEX.exec) or explicitly reset
YOUTUBE_REGEX.lastIndex = 0 immediately before calling YOUTUBE_REGEX.exec(href);
apply the same fix to the other occurrence around the code that references
match/e and YOUTUBE_REGEX.
- Around line 528-547: The code calls href.match(VIMEO_REGEX) into match but
then calls VIMEO_REGEX.exec(href) (assigned to e), which can fail due to regex
lastIndex when the pattern is global; fix by reusing the earlier match result
(use match[3] instead of calling VIMEO_REGEX.exec) or by resetting
VIMEO_REGEX.lastIndex = 0 before exec; update the block around
match/VIMEO_REGEX.exec so you either replace e with the existing match variable
or ensure lastIndex is cleared to reliably get the capture group (reference
VIMEO_REGEX, match, e, and href).
- Around line 707-741: The code calls SPEAK_REGEX.exec(href) after already
computing match = href.match(SPEAK_REGEX), which risks incorrect results when
SPEAK_REGEX has the global flag because lastIndex is stateful; fix by reusing
the existing match array (use match[1], match[2], match[3] instead of e) or, if
you must call exec, reset SPEAK_REGEX.lastIndex = 0 before calling; update the
block that declares const e = SPEAK_REGEX.exec(href) to use match or reset
lastIndex to prevent intermittent failures.
- Around line 550-580: TWITCH_REGEX is being used with both String.match and
RegExp.exec which causes flaky behavior when TWITCH_REGEX is a global regex
because RegExp.lastIndex can be non-zero; fix by resetting
TWITCH_REGEX.lastIndex = 0 before calling TWITCH_REGEX.exec(href) or by
eliminating the extra exec and reuse the earlier match result (use the captured
groups from match instead of calling exec), updating references to e accordingly
so embedSrc logic uses the reliable capture groups.
- Around line 471-487: The code calls BRIGHTEON_REGEX.exec(href) after already
doing const BNmatch = href.match(BRIGHTEON_REGEX), which can suffer from
RegExp.lastIndex state; replace the exec call with the previously computed
BNmatch (e.g. const e = BNmatch) or reset BRIGHTEON_REGEX.lastIndex = 0 before
exec so the match is deterministic; update the block that sets const e =
BRIGHTEON_REGEX.exec(href) to use BNmatch (or reset lastIndex) and keep the rest
of the logic (vid, embedSrc, DOM updates) unchanged.
🧹 Nitpick comments (9)
packages/render-helper/src/post-body-summary.ts (4)
10-32: Consider extracting magic number10as a named constant.The hardcoded tolerance value
10in the length check could be made more self-documenting.♻️ Suggested improvement
+const WORD_OVERFLOW_TOLERANCE = 10; + const joint = (arr: string[], limit = 200) => { let result = ''; if (arr) { for (let i = 0; i < arr.length; i++) { // join array with space separator if (result) { result += " "; } // break with length reaches limit if (result.length > limit) { break; } else { // make sure last join doesn't break the limit too much - if ((result + arr[i]).length < limit + 10) { + if ((result + arr[i]).length < limit + WORD_OVERFLOW_TOLERANCE) { result += arr[i]; } else { break; } } } } return result.trim(); };
67-80: Entity placeholder collision edge case.The placeholder pattern
\u200B${index}\u200Bcould theoretically collide if the original text contains the same pattern (e.g.,\u200B0\u200B). This is highly unlikely in practice, but for robustness, consider using a more unique delimiter.♻️ More robust placeholder pattern
if (entities && platform !== 'web') { // Deduplicate entities to avoid duplicate placeholders const uniqueEntities = [...new Set(entities)]; uniqueEntities.forEach((entity, index) => { - // Use markdown-inert Unicode placeholder (zero-width spaces) - const placeholder = `\u200B${index}\u200B`; + // Use markdown-inert Unicode placeholder with unique prefix + const placeholder = `\u200B__ENTITY_${index}__\u200B`; entityPlaceholders.push(entity); // Replace all occurrences of this entity entryBody = entryBody.split(entity).join(placeholder); }) }Also update the restoration logic at line 101 to match.
108-113: Consider simplifying the multiple-spaces regex.The positive lookahead regex
/ +(?= )/gworks but could be more readable.♻️ Simplified regex
text = text .replace(/(<([^>]+)>)/gi, '') // Remove html tags .replace(/\r?\n|\r/g, ' ') // Remove new lines .replace(/(?:https?|ftp):\/\/[\n\S]+/g, '') // Remove urls .trim() - .replace(/ +(?= )/g, '') // Remove all multiple spaces + .replace(/ {2,}/g, ' ') // Collapse multiple spaces to single space
140-142: Redundant type cast.The
as stringcast is unnecessary since TypeScript narrows the type after thetypeof obj === 'string'check.♻️ Remove redundant cast
if (typeof obj === 'string') { - return postBodySummary(obj as string, normalizedLength, normalizedPlatform) + return postBodySummary(obj, normalizedLength, normalizedPlatform) }packages/wallets/src/modules/assets/hive/mutations/delegate.ts (1)
8-14: Consider removing unusedmemofield from Payload interface.The
memofield in thePayloadinterface is not used in thedelegate_vesting_sharesoperation (lines 19-23). If this interface is specific to this file, consider removing the unused field for clarity.Suggested change
interface Payload<T extends HiveBasedAssetSignType> { from: string; to: string; amount: string; - memo: string; type: T; }packages/wallets/src/modules/wallets/queries/get-account-wallet-asset-info-query-options.ts (1)
95-121: Solid implementation for parsing extraData.The logic correctly handles formatted strings with commas and extracts numeric values. One observation:
Duplicate entries: If
extraDatacontains multiple entries with the samedataKey, this will push multiple parts with the same name. Depending on your API's behavior and how downstream consumers handle the parts array, consider deduplicating by dataKey using a Map if multiple entries for the same key should be merged rather than kept separate.The type definition for
extraData(Array<{ dataKey: string; value: any }>) is already properly defined upstream inget-vision-portfolio-query-options.ts, and the defensive null/type checks here provide good runtime safety.packages/render-helper/src/methods/markdown-to-html.method.ts (3)
24-29: Use regex literals instead ofnew RegExp()for static patterns.Creating
RegExpobjects withnew RegExp("...", "g")on every call is less efficient and harder to read. Since these patterns are static, use regex literals.♻️ Suggested refactor
- input = input.replace(new RegExp("https://leofinance.io/threads/view/","g"), "/@"); - input = input.replace(new RegExp("https://leofinance.io/posts/","g"), "/@"); - input = input.replace(new RegExp("https://leofinance.io/threads/","g"), "/@"); - input = input.replace(new RegExp("https://inleo.io/threads/view/","g"), "/@"); - input = input.replace(new RegExp("https://inleo.io/posts/","g"), "/@"); - input = input.replace(new RegExp("https://inleo.io/threads/","g"), "/@"); + input = input.replace(/https:\/\/leofinance\.io\/threads\/view\//g, "/@"); + input = input.replace(/https:\/\/leofinance\.io\/posts\//g, "/@"); + input = input.replace(/https:\/\/leofinance\.io\/threads\//g, "/@"); + input = input.replace(/https:\/\/inleo\.io\/threads\/view\//g, "/@"); + input = input.replace(/https:\/\/inleo\.io\/posts\//g, "/@"); + input = input.replace(/https:\/\/inleo\.io\/threads\//g, "/@");
69-71: Move empty input check before parser initialization.The early return for empty input is placed after initializing the
Remarkableparser andXMLSerializer. Moving this check to the beginning of the function avoids unnecessary object construction.♻️ Suggested refactor
export function markdownToHTML(input: string, forApp: boolean, webp: boolean): string { + if (!input) { + return '' + } + // Internalize leofinance.io links input = input.replace(new RegExp("https://leofinance.io/threads/view/","g"), "/@"); // ... rest of replacements ... const md = new Remarkable({ // ... const serializer = new XMLSerializer() - if (!input) { - return '' - } - let output = '';
109-123: Re-rendering markdown is redundant; use the already-rendered output.Line 109 calls
md.render(input)again, butoutputalready contains the rendered result from line 93. Although we're in the catch block,outputwas assigned before the error could occur duringDOMParser.parseFromString. Use the existingoutputvalue to avoid redundant rendering.♻️ Suggested refactor
} catch (error) { // @xmldom/xmldom is stricter than old xmldom and throws ParseError for malformed HTML // Instead of returning empty string (bad UX), attempt graceful recovery try { // Strategy: // 1. Pre-sanitize to remove XSS vectors // 2. Use lenient htmlparser2 to auto-repair malformed HTML (auto-close unclosed tags) // 3. Serialize the repaired HTML // 4. Re-parse with @xmldom/xmldom (now well-formed) // 5. Traverse and serialize as normal - output = md.render(input) const preSanitized = sanitizeHtml(output)
📜 Review details
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (13)
packages/render-helper/dist/browser/index.d.tsis excluded by!**/dist/**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/**,!**/*.mappackages/wallets/dist/browser/index.jsis excluded by!**/dist/**packages/wallets/dist/browser/index.js.mapis excluded by!**/dist/**,!**/*.mappackages/wallets/dist/node/index.cjsis excluded by!**/dist/**packages/wallets/dist/node/index.cjs.mapis excluded by!**/dist/**,!**/*.mappackages/wallets/dist/node/index.mjsis excluded by!**/dist/**packages/wallets/dist/node/index.mjs.mapis excluded by!**/dist/**,!**/*.map
📒 Files selected for processing (24)
apps/web/src/app/(dynamicPages)/profile/[username]/wallet/(token)/[token]/_components/hive-engine-claim-rewards-button.tsxapps/web/src/features/ui/list/index.tsxapps/web/src/features/wallet/operations/wallet-operations-sign.tsxpackages/render-helper/src/methods/a.method.tspackages/render-helper/src/methods/markdown-to-html.method.tspackages/render-helper/src/post-body-summary.tspackages/renderer/package.jsonpackages/wallets/src/modules/assets/hive-engine/mutations/broadcast-hive-engine-operation.tspackages/wallets/src/modules/assets/hive-engine/mutations/claim-rewards.tspackages/wallets/src/modules/assets/hive/mutations/claim-interest.tspackages/wallets/src/modules/assets/hive/mutations/convert.tspackages/wallets/src/modules/assets/hive/mutations/delegate.tspackages/wallets/src/modules/assets/hive/mutations/power-down.tspackages/wallets/src/modules/assets/hive/mutations/power-up.tspackages/wallets/src/modules/assets/hive/mutations/transfer-from-savings.tspackages/wallets/src/modules/assets/hive/mutations/transfer-to-savings.tspackages/wallets/src/modules/assets/hive/mutations/transfer.tspackages/wallets/src/modules/assets/hive/mutations/withdraw-route.tspackages/wallets/src/modules/assets/points/mutations/gift.tspackages/wallets/src/modules/assets/spk/mutations/lock.tspackages/wallets/src/modules/assets/spk/mutations/power-up.tspackages/wallets/src/modules/assets/spk/mutations/transfer-larynx.tspackages/wallets/src/modules/assets/spk/mutations/transfer.tspackages/wallets/src/modules/wallets/queries/get-account-wallet-asset-info-query-options.ts
🚧 Files skipped from review as they are similar to previous changes (1)
- packages/renderer/package.json
🧰 Additional context used
🧬 Code graph analysis (13)
apps/web/src/app/(dynamicPages)/profile/[username]/wallet/(token)/[token]/_components/hive-engine-claim-rewards-button.tsx (1)
packages/wallets/src/modules/assets/hive-engine/mutations/claim-rewards.ts (1)
claimHiveEngineRewards(14-52)
packages/wallets/src/modules/wallets/queries/get-account-wallet-asset-info-query-options.ts (2)
packages/wallets/dist/node/index.cjs (11)
assetInfo(3655-3657)assetInfo(3711-3711)dataKey(3672-3672)value(3056-3056)value(3673-3673)cleanValue(3675-3675)match(2989-2989)match(3676-3676)numValue(3678-3678)parts(2257-2260)parts(3659-3659)packages/wallets/dist/browser/index.js (2)
value(3031-3031)match(2964-2964)
packages/wallets/src/modules/assets/hive/mutations/power-down.ts (3)
packages/wallets/dist/node/index.mjs (1)
auth(373-373)packages/wallets/dist/browser/index.d.ts (1)
broadcastWithWalletHiveAuth(2031-2031)packages/wallets/src/modules/assets/utils/hive-auth.ts (1)
broadcastWithWalletHiveAuth(16-27)
packages/wallets/src/modules/assets/spk/mutations/power-up.ts (1)
packages/wallets/src/modules/assets/utils/hive-auth.ts (1)
broadcastWithWalletHiveAuth(16-27)
packages/wallets/src/modules/assets/hive/mutations/transfer.ts (1)
packages/wallets/src/modules/assets/utils/hive-auth.ts (1)
broadcastWithWalletHiveAuth(16-27)
packages/wallets/src/modules/assets/hive/mutations/withdraw-route.ts (3)
packages/wallets/dist/node/index.mjs (1)
auth(373-373)packages/wallets/dist/browser/index.d.ts (1)
broadcastWithWalletHiveAuth(2031-2031)packages/wallets/src/modules/assets/utils/hive-auth.ts (1)
broadcastWithWalletHiveAuth(16-27)
packages/wallets/src/modules/assets/hive-engine/mutations/broadcast-hive-engine-operation.ts (3)
packages/wallets/dist/node/index.cjs (1)
auth(400-400)packages/wallets/dist/node/index.mjs (1)
auth(373-373)packages/wallets/dist/browser/index.d.ts (1)
broadcastWithWalletHiveAuth(2031-2031)
packages/wallets/src/modules/assets/hive/mutations/transfer-from-savings.ts (3)
packages/wallets/dist/node/index.mjs (1)
auth(373-373)packages/wallets/dist/browser/index.d.ts (1)
broadcastWithWalletHiveAuth(2031-2031)packages/wallets/src/modules/assets/utils/hive-auth.ts (1)
broadcastWithWalletHiveAuth(16-27)
packages/wallets/src/modules/assets/spk/mutations/lock.ts (3)
packages/wallets/dist/node/index.mjs (1)
auth(373-373)packages/wallets/dist/browser/index.d.ts (1)
broadcastWithWalletHiveAuth(2031-2031)packages/wallets/src/modules/assets/utils/hive-auth.ts (1)
broadcastWithWalletHiveAuth(16-27)
packages/wallets/src/modules/assets/hive/mutations/power-up.ts (3)
packages/wallets/dist/node/index.mjs (1)
auth(373-373)packages/wallets/dist/browser/index.d.ts (1)
broadcastWithWalletHiveAuth(2031-2031)packages/wallets/src/modules/assets/utils/hive-auth.ts (1)
broadcastWithWalletHiveAuth(16-27)
packages/wallets/src/modules/assets/hive/mutations/delegate.ts (4)
packages/wallets/dist/node/index.cjs (1)
auth(400-400)packages/wallets/dist/node/index.mjs (1)
auth(373-373)packages/wallets/dist/browser/index.d.ts (1)
broadcastWithWalletHiveAuth(2031-2031)packages/wallets/src/modules/assets/utils/hive-auth.ts (1)
broadcastWithWalletHiveAuth(16-27)
packages/wallets/src/modules/assets/spk/mutations/transfer.ts (2)
packages/wallets/dist/node/index.mjs (1)
auth(373-373)packages/wallets/src/modules/assets/utils/hive-auth.ts (1)
broadcastWithWalletHiveAuth(16-27)
packages/wallets/src/modules/assets/hive/mutations/convert.ts (4)
packages/wallets/dist/browser/index.js (26)
payload(1326-1326)payload(1384-1384)payload(1411-1411)payload(1556-1556)payload(1648-1648)payload(1697-1697)payload(1742-1742)payload(1791-1791)payload(2836-2836)auth(375-375)operation(1247-1255)operation(1293-1293)operation(1324-1324)operation(1354-1354)operation(1382-1382)operation(1409-1409)operation(1438-1438)operation(1581-1581)operation(1638-1646)operation(1687-1695)operation(1732-1740)operation(1781-1789)operation(2323-2339)operation(2371-2387)operation(2419-2435)operation(2467-2483)packages/wallets/dist/node/index.cjs (8)
payload(1351-1351)payload(1409-1409)payload(1436-1436)payload(1581-1581)payload(1673-1673)payload(1722-1722)payload(1767-1767)auth(400-400)packages/wallets/dist/node/index.mjs (1)
auth(373-373)packages/wallets/dist/browser/index.d.ts (1)
broadcastWithWalletHiveAuth(2031-2031)
🔇 Additional comments (31)
packages/wallets/src/modules/assets/hive/mutations/claim-interest.ts (2)
52-57: LGTM!The strict error handling for keychain is appropriate since keychain operations require an external broadcaster to be provided. The explicit error message will help with debugging.
59-64: LGTM!The fallback to
broadcastWithWalletHiveAuthis appropriate here since HiveAuth has built-in broadcasting capability, unlike keychain which relies on an external broadcaster. The asymmetric handling between keychain (throws) and hiveauth (fallback) correctly reflects the different capabilities of each authentication method.apps/web/src/features/ui/list/index.tsx (1)
11-11: LGTM!Adding
"grid"to the filter list correctly prevents the customgridprop from being spread onto the native<div>element, which would otherwise cause React warnings about unknown DOM attributes.packages/wallets/src/modules/assets/points/mutations/gift.ts (2)
50-55: LGTM!The keychain handling is clean and correctly throws when the required broadcaster is missing.
57-62: LGTM! Intentional fallback design for hiveauth.The new hiveauth branch correctly prioritizes the provided
auth.broadcastwhen available and gracefully falls back tobroadcastWithWalletHiveAuthotherwise. This differs from the keychain path (which throws on missing auth) but appears intentional since hiveauth has a dedicated fallback mechanism available.packages/wallets/src/modules/assets/spk/mutations/lock.ts (2)
44-48: LGTM!The keychain branch is now cleanly separated with its existing behavior preserved—requiring an explicit broadcaster and throwing if unavailable.
49-53: LGTM!The dedicated hiveauth branch properly prioritizes
auth.broadcastwhen available and falls back to the specializedbroadcastWithWalletHiveAuthhandler. This is a sensible design since hiveauth has its own registered broadcast mechanism, unlike keychain which must rely on an external broadcaster.packages/wallets/src/modules/assets/spk/mutations/transfer-larynx.ts (2)
49-53: LGTM!The keychain branch now explicitly handles only the keychain type with clear behavior: use broadcaster if available, otherwise throw an error. This separation from hiveauth improves code clarity.
54-58: LGTM!The new hiveauth branch correctly prioritizes the broadcaster when available and falls back to
broadcastWithWalletHiveAuthotherwise. This is the right approach since HiveAuth can operate independently through its own authentication flow, unlike keychain which requires the browser extension.apps/web/src/app/(dynamicPages)/profile/[username]/wallet/(token)/[token]/_components/hive-engine-claim-rewards-button.tsx (1)
153-181: LGTM! Clean refactoring to use activeUser context directly.The changes appropriately simplify the authentication flow by using
activeUserfrom the global context instead of re-fetching viagetUser(cleanUsername). Key points:
- The guard at line 155 correctly ensures
activeUserexists before accessing its properties.- Since
canClaimalready validatesisOwnProfile(ensuringcleanUsername === activeUser.username), this approach is safe.- The
PrivateKey.fromString()call at line 174 could throw if the stored key is corrupted, but theonErrorhandler will catch it.packages/render-helper/src/post-body-summary.ts (3)
1-8: LGTM!Imports are well-organized and appropriate for the module's functionality.
82-95: Good error handling with contextual logging.The try/catch around markdown rendering with structured error logging is a solid defensive pattern. Returning empty string on failure provides graceful degradation.
144-154: LGTM! Well-structured caching implementation.The cache key correctly incorporates length and platform parameters, ensuring different configurations don't return stale results. The early return pattern for cache hits is clean and efficient.
packages/wallets/src/modules/assets/hive/mutations/power-down.ts (1)
30-42: LGTM! Clean separation of keychain and hiveauth handling.The refactored branching logic correctly separates the authentication methods:
- Keychain requires an explicit broadcaster (throws if missing)
- HiveAuth has a fallback to
broadcastWithWalletHiveAuthwhen no broadcaster is provided- Default path falls through to hivesigner
This pattern is consistent with the other mutation files in this PR.
packages/wallets/src/modules/assets/hive/mutations/transfer-from-savings.ts (1)
47-61: LGTM! Consistent refactoring pattern.The separation of keychain and hiveauth branches follows the same pattern as other mutation files. The use of standalone
ifblocks with early returns is clean and readable.packages/wallets/src/modules/assets/hive-engine/mutations/claim-rewards.ts (1)
33-45: LGTM! Correct key type usage.The refactoring correctly uses
"posting"as the key type, which aligns with the operation usingrequired_posting_authsat line 24. The hiveauth fallback pattern is consistent with other mutations.packages/wallets/src/modules/assets/hive/mutations/transfer.ts (1)
52-69: LGTM! Clean separation maintained.The refactored branches correctly handle the transfer operation:
- Keychain path requires explicit broadcaster
- HiveAuth path gracefully falls back to
broadcastWithWalletHiveAuth- Hivesigner fallback preserved in the else block
The pattern is consistent with the other wallet mutations in this PR.
packages/wallets/src/modules/assets/hive/mutations/withdraw-route.ts (1)
38-53: LGTM! Correctly adapts to the payload structure.The hiveauth fallback correctly uses
payload.from_account(matching this file's payload interface) rather thanpayload.fromused in other mutations. The refactoring pattern is consistent with the other wallet mutation files.packages/wallets/src/modules/assets/hive-engine/mutations/broadcast-hive-engine-operation.ts (1)
11-23: LGTM! Clean separation of keychain and hiveauth branches.The refactored control flow correctly handles both signing types:
keychainrequires an external broadcaster and appropriately throws when missinghiveauthproperly falls back tobroadcastWithWalletHiveAuthwhen no broadcaster is configuredThis aligns with the broader wallet mutation patterns across the codebase.
packages/wallets/src/modules/assets/spk/mutations/power-up.ts (1)
44-53: LGTM! Consistent refactoring pattern applied.The keychain/hiveauth split follows the same established pattern:
keychainthrows when broadcaster is missinghiveauthfalls back tobroadcastWithWalletHiveAuthThis maintains consistency with other wallet mutations in the PR.
packages/wallets/src/modules/assets/hive/mutations/power-up.ts (1)
33-42: LGTM! Matches the established refactoring pattern.The keychain/hiveauth branching is consistent with other mutation files in this PR, properly routing hiveauth to its dedicated fallback when no broadcaster is available.
packages/wallets/src/modules/assets/hive/mutations/delegate.ts (1)
32-41: LGTM! Consistent keychain/hiveauth separation.The refactoring correctly implements the two-branch pattern for keychain and hiveauth, matching the approach used across other wallet mutations.
packages/wallets/src/modules/assets/spk/mutations/transfer.ts (1)
49-58: LGTM! Consistent implementation of keychain/hiveauth branching.The refactoring correctly separates keychain and hiveauth handling, with hiveauth appropriately falling back to
broadcastWithWalletHiveAuthwhen no broadcaster is configured. This aligns with the pattern established across all wallet mutation files in this PR.packages/wallets/src/modules/assets/hive/mutations/transfer-to-savings.ts (1)
34-43: LGTM! Clean separation of keychain and hiveauth branches.The refactor properly isolates keychain and hiveauth handling with appropriate fallback behavior. The hiveauth branch correctly falls back to
broadcastWithWalletHiveAuthwhen no broadcaster is present, while the keychain branch throws an error as expected.packages/wallets/src/modules/assets/hive/mutations/convert.ts (1)
32-41: LGTM! Consistent with the refactor pattern.This follows the same keychain/hiveauth separation pattern as the other mutation files in the PR, ensuring consistent behavior across all wallet operations.
apps/web/src/features/wallet/operations/wallet-operations-sign.tsx (1)
45-84: Good centralization of broadcast logic.The memoized
authContextproperly unifies the signing flow for different authentication methods. The dependency array correctly includes all reactive values (user,activeUser,useHiveAuth,hasKeyChain).packages/render-helper/src/methods/markdown-to-html.method.ts (2)
140-147: LGTM on entity restoration logic.The placeholder restoration correctly iterates through
entityPlaceholdersin the same order used during replacement, ensuring proper reconstruction of the original entities.
113-114: Incorrect characterization of code placement and bundling concerns.
htmlparser2anddom-serializerare required in the try block (lines 113-114), not catch blocks. Onlyheis in the catch block. More importantly, all three packages (htmlparser2,dom-serializer,he) are explicitly declared as dependencies in package.json and will be bundled by the build system. Dynamic requires at runtime are a legitimate pattern in Node.js codebases and do not pose a browser bundling issue when the packages are listed as dependencies. The lazy-load pattern used forlolightis unnecessary here since these dependencies are already explicitly declared.Likely an incorrect or invalid review comment.
packages/render-helper/src/methods/a.method.ts (3)
743-772: Good use of DOM construction for Twitter embeds to prevent XSS.Using
createElement,setAttribute, andtextContentinstead of string concatenation properly escapes user-controlled values (url,author) and prevents XSS vulnerabilities. This is a solid approach.
76-80: XSS prevention forjavascript:URLs is correct.The check correctly uses
trim().toLowerCase()before comparing, preventing bypass attempts with whitespace or mixed-case likeJaVaScRiPt:.
453-469: No action required. RUMBLE_REGEX does not have the global flag and does not requirelastIndexto be reset. Non-global regexes ignore thelastIndexproperty entirely, making the proposed fix unnecessary.Likely an incorrect or invalid review comment.
apps/web/src/features/wallet/operations/wallet-operations-sign.tsx
Outdated
Show resolved
Hide resolved
There was a problem hiding this comment.
Actionable comments posted: 6
🤖 Fix all issues with AI agents
In @packages/render-helper/src/methods/a.method.ts:
- Around line 570-593: The code risks a null dereference by calling
SPOTIFY_REGEX.exec(href) into e and then accessing e[1]; replace that exec usage
with the previously obtained match (use match[1]) or add a null-check for e
before accessing e[1]; also remove or keep the SPOTIFY_REGEX.lastIndex reset as
needed but ensure you rely on the validated match variable (from match =
href.match(SPOTIFY_REGEX)) when building embedSrc and appending the iframe so
there is no possibility of e being null.
- Around line 453-469: The code calls RUMBLE_REGEX.exec(href) and dereferences
e[1] without ensuring exec didn't return null; if RUMBLE_REGEX is global exec
can return null due to lastIndex. Fix by reusing the already-successful RBmatch
(use RBmatch[1] as vid) or, if you must call exec, reset RUMBLE_REGEX.lastIndex
= 0 before exec and add a null check on the result (ensure e is non-null before
accessing e[1]) when setting vid/embedSrc and manipulating el.
- Around line 595-617: Update the incorrect comment and eliminate the null-deref
by not calling LOOM_REGEX.exec() again; use the already matched result or guard
the exec result before indexing. Specifically, change the comment "If a spotify
audio" to something like "If a Loom video", and replace const e =
LOOM_REGEX.exec(href) / subsequent e[2] access with a safe use of match (e.g.,
const id = match[2]; if (id) { ... }) or check that e is non-null before using
e[2] in the block that builds embedSrc and the iframe.
- Around line 670-692: The code risks a null dereference by calling
D_TUBE_REGEX2.exec(href) into e; reuse the already-obtained match variable
instead of exec — check match[2] and match[3] (the username and object id),
remove the exec() call and the lastIndex reset, and construct videoHref from
match[2]/match[3]; update attribute setting and return as before.
- Around line 619-669: D_TUBE_REGEX.exec(href) can return null so the code may
dereference e (e[2]/e[3]) causing a crash; replace the exec usage with the
existing match result (use match[2] and match[3]) or add a null guard before
checking e (e && e[2] && e[3]) and only call D_TUBE_REGEX.exec if you need it,
remembering to reset D_TUBE_REGEX.lastIndex when using exec; update the block
around D_TUBE_REGEX.exec(href) so it safely reads the capture groups from match
or checks e for null before accessing indexes.
🧹 Nitpick comments (2)
apps/web/src/features/wallet/operations/wallet-operations-sign.tsx (1)
43-89: Well-structured auth context extension.The logic correctly extends
getSdkAuthContextwith broadcast methods for users who logged in via other means (e.g., private key or hivesigner) but can still use keychain/HiveAuth for signing. The priority order (HiveAuth → Keychain → error) and authority mappings are consistent with the existinggetSdkAuthContextimplementation.Minor improvement: Line 83 uses
(result: any)which loses type safety.♻️ Optional: Add proper typing for keychain result
return keychain .broadcast(activeUser.username, operations, keychainAuthority) - .then((result: any) => result.result); + .then((result) => (result as { result: unknown }).result);Or if
keychain.broadcasthas proper return types defined, consider using them directly.packages/render-helper/src/methods/a.method.ts (1)
58-814: Consider extracting video embed handlers into separate functions.The
a()function handles ~20 different URL patterns across 750+ lines. Extracting handlers for video embeds (YouTube, Vimeo, Twitch, DTube, etc.) into separate functions would improve readability and testability.This is optional and can be deferred to a follow-up refactor.
📜 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 (2)
apps/web/src/features/wallet/operations/wallet-operations-sign.tsxpackages/render-helper/src/methods/a.method.ts
🧰 Additional context used
🧬 Code graph analysis (1)
apps/web/src/features/wallet/operations/wallet-operations-sign.tsx (4)
packages/sdk/dist/index.d.ts (2)
getUser(1106-1106)keychain(1106-1106)packages/sdk/dist/browser/index.d.ts (1)
AuthContext(3194-3194)apps/web/src/utils/sdk-auth.ts (1)
getSdkAuthContext(6-46)apps/web/src/utils/hive-auth.ts (1)
broadcastWithHiveAuth(1374-1381)
🔇 Additional comments (2)
apps/web/src/features/wallet/operations/wallet-operations-sign.tsx (2)
91-100: LGTM!The
authContextis correctly passed touseWalletOperation. The hook should handleundefinedauth context appropriately, which aligns with the existing pattern fromgetSdkAuthContext.
186-209: LGTM!The button correctly routes to "hiveauth" or "keychain" signing type based on the
useHiveAuthflag, which is consistent with the priority order in theauthContext.broadcastmethod.
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (5)
packages/render-helper/src/methods/a.method.ts (5)
88-96: Consider stricter node selection for image replacement.The fallback
doc.firstChildcould potentially return unexpected node types (doctype, comment). Consider usingdoc.querySelector('img')or explicitly checking the node type.♻️ Suggested improvement
- const replaceNode = doc.body?.firstChild || doc.firstChild + const replaceNode = doc.body?.firstChild ?? doc.documentElement?.firstChild if (replaceNode) { el.parentNode.replaceChild(replaceNode, el) }
440-486: Consider extracting common video embed logic.The handlers for Bitchute, Rumble, and Brighteon follow nearly identical patterns. This duplication could be reduced with a helper function.
♻️ Example refactoring approach
// Helper function for simple video embeds function createSimpleVideoEmbed( el: HTMLElement, embedSrc: string, className: string = 'markdown-video-link' ): void { el.setAttribute('class', className) el.removeAttribute('href') el.textContent = '' el.setAttribute('data-embed-src', embedSrc) const play = el.ownerDocument.createElement('span') play.setAttribute('class', 'markdown-video-play') el.appendChild(play) } // Usage: const BCmatch = href.match(BITCHUTE_REGEX) if (BCmatch && BCmatch[1] && el.textContent.trim() === href) { createSimpleVideoEmbed(el, `https://www.bitchute.com/embed/${BCmatch[1]}/`) return }This would also apply to Rumble, Brighteon, and partially to YouTube, Vimeo, etc.
795-798: Addnoreferrerto external link rel attribute.For external links, best practice is to include both
noopenerandnoreferrerto prevent the target page from accessingwindow.openerand from receiving the referrer header.♻️ Proposed fix
} else { el.setAttribute('target', '_blank'); - el.setAttribute('rel', 'noopener'); + el.setAttribute('rel', 'noopener noreferrer'); }
58-800: Consider decomposing this function for maintainability.The
a()function is ~740 lines handling 20+ link types. While the sequential if-return pattern is clear, consider extracting link-type-specific handlers into separate functions (e.g.,handleYouTubeLink(),handleHivePost(),handleTwitterEmbed()). This would improve testability and make individual handlers easier to maintain.This is a lower-priority improvement that could be addressed in future iterations.
533-537: Consider adding sandbox attributes to Vimeo and Twitch iframes.Spotify and Loom iframes include
sandboxattributes, but Vimeo and Twitch do not. For consistency and defense-in-depth, consider applying sandboxing to all third-party iframe embeds.♻️ Example for Vimeo
const ifr = el.ownerDocument.createElement('iframe') ifr.setAttribute('frameborder', '0') ifr.setAttribute('allowfullscreen', 'true') ifr.setAttribute('src', embedSrc) + ifr.setAttribute('sandbox', 'allow-scripts allow-same-origin allow-popups') el.appendChild(ifr)Note: Test that the embeds function correctly with sandboxing, as some may require additional permissions.
Also applies to: 563-567
📜 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 (1)
packages/render-helper/src/methods/a.method.ts
🔇 Additional comments (4)
packages/render-helper/src/methods/a.method.ts (4)
58-61: LGTM on the guard clauses.Good defensive programming with early returns for null element and missing parentNode.
76-80: Good XSS prevention for javascript: protocol.Proper handling with case-insensitive check and complete href removal.
716-745: Good secure DOM construction for Twitter embeds.Using
textContentandsetAttributefor automatic escaping is the correct approach to prevent XSS.
754-767: LGTM on error handling for malformed URIs.Good defensive coding with try-catch for
decodeURI. The silent catch is acceptable since invalid URIs should just skip processing.
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 Fix all issues with AI agents
In @packages/render-helper/src/methods/iframe.method.ts:
- Around line 33-38: Twitch embed URL assembly in the TWITCH_EMBED_REGEX branch
can produce malformed URLs when src has no query string; update the logic in the
if (src.match(TWITCH_EMBED_REGEX)) block to check whether src already contains a
'?' and append parameters with '?' if not or '&' if yes (use the existing
variables src and parentDomain), then set el.setAttribute('src', s) with the
correctly joined string.
🧹 Nitpick comments (9)
packages/render-helper/src/methods/markdown-to-html.method.ts (4)
8-20: Consider dynamicimport()for broader bundler compatibility.The
requirecall may fail in ESM-only environments or certain bundlers (e.g., Vite, esbuild in ESM mode). Consider using dynamicimport()with top-level await or an async initialization pattern if this package needs to support pure ESM consumers.The current fallback handling is correct for runtime availability, so this is fine if CommonJS is guaranteed.
22-30: Extract static regex patterns as module-level constants.Creating 6 new
RegExpobjects on every function call is inefficient. Since these patterns never change, define them once at module scope.♻️ Suggested refactor
+// Module-level constants for URL normalization +const LEOFINANCE_THREADS_VIEW_REGEX = /https:\/\/leofinance\.io\/threads\/view\//g; +const LEOFINANCE_POSTS_REGEX = /https:\/\/leofinance\.io\/posts\//g; +const LEOFINANCE_THREADS_REGEX = /https:\/\/leofinance\.io\/threads\//g; +const INLEO_THREADS_VIEW_REGEX = /https:\/\/inleo\.io\/threads\/view\//g; +const INLEO_POSTS_REGEX = /https:\/\/inleo\.io\/posts\//g; +const INLEO_THREADS_REGEX = /https:\/\/inleo\.io\/threads\//g; + export function markdownToHTML(input: string, forApp: boolean, webp: boolean, parentDomain: string = 'ecency.com'): string { // Internalize leofinance.io links - input = input.replace(new RegExp("https://leofinance.io/threads/view/","g"), "/@"); - input = input.replace(new RegExp("https://leofinance.io/posts/","g"), "/@"); - input = input.replace(new RegExp("https://leofinance.io/threads/","g"), "/@"); - input = input.replace(new RegExp("https://inleo.io/threads/view/","g"), "/@"); - input = input.replace(new RegExp("https://inleo.io/posts/","g"), "/@"); - input = input.replace(new RegExp("https://inleo.io/threads/","g"), "/@"); + input = input.replace(LEOFINANCE_THREADS_VIEW_REGEX, "/@"); + input = input.replace(LEOFINANCE_POSTS_REGEX, "/@"); + input = input.replace(LEOFINANCE_THREADS_REGEX, "/@"); + input = input.replace(INLEO_THREADS_VIEW_REGEX, "/@"); + input = input.replace(INLEO_POSTS_REGEX, "/@"); + input = input.replace(INLEO_THREADS_REGEX, "/@");
75-89: Minor collision risk with zero-width space placeholders.The placeholder pattern
\u200B{index}\u200Bcould theoretically collide with user content containing the same sequence. While rare, consider using a more unique delimiter (e.g., a longer or randomized prefix) for robustness.The deduplication and replacement logic is otherwise sound.
112-137: Move dynamicrequirestatements to top-level imports.The inline
requirecalls forhtmlparser2,dom-serializer, andhe(lines 113-114, 134) will cause issues with tree-shaking and may fail in ESM environments. Since these are used in error recovery paths, they should still be bundled.♻️ Suggested refactor
Add to top-level imports:
import * as htmlparser2 from 'htmlparser2' import domSerializer from 'dom-serializer' import he from 'he'Then remove the inline
requirecalls:- const htmlparser2 = require('htmlparser2') - const domSerializer = require('dom-serializer').default const dom = htmlparser2.parseDocument(preSanitized, {- const he = require('he') - const escapedContent = he.encode(output || md.render(input)) + const escapedContent = he.encode(output || md.render(input))packages/render-helper/src/methods/iframe.method.ts (1)
140-145: Typo in comment: "Brigtheon" should be "Brighteon".✏️ Fix typo
- // Brigtheon + // Brighteon if (src.match(BRIGHTEON_REGEX)) {packages/render-helper/src/methods/traverse.method.ts (2)
12-13: Simplify child node collection.The current approach
Array.from(Array(n).keys()).map(i => ...)is unnecessarily complex. UseArray.from(node.childNodes)directly for clarity and slight performance improvement.♻️ Simplified child collection
- Array.from(Array(node.childNodes.length).keys()) - .map(i => node.childNodes[i]) - .forEach(child => { + Array.from(node.childNodes).forEach(child => {
21-23: Incorrect type cast for text nodes.Text nodes (
#text) are of typeText, notHTMLElement. The cast works at runtime becausetext()only usesNode-level properties, but this is misleading. Consider updating thetext()function signature to acceptNode | nullor cast toText.packages/render-helper/src/methods/a.method.ts (2)
58-61: UnusedparentDomainparameter.The
parentDomainparameter is declared but never used within the function body. If this is intentional for API consistency with other methods likeiframe(), consider adding a comment. Otherwise, remove it.
534-538: Consider adding sandbox attribute to dynamically created iframes.The Vimeo iframe created here lacks a
sandboxattribute, whileiframe.method.tsdoesn't add one either for Vimeo. Consider whether sandbox restrictions (likeallow-scripts allow-same-origin) should be applied consistently for security hardening.
📜 Review details
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (7)
packages/render-helper/dist/browser/index.d.tsis excluded by!**/dist/**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 (5)
packages/render-helper/src/markdown-2-html.tspackages/render-helper/src/methods/a.method.tspackages/render-helper/src/methods/iframe.method.tspackages/render-helper/src/methods/markdown-to-html.method.tspackages/render-helper/src/methods/traverse.method.ts
🚧 Files skipped from review as they are similar to previous changes (1)
- packages/render-helper/src/markdown-2-html.ts
🧰 Additional context used
🧬 Code graph analysis (3)
packages/render-helper/src/methods/traverse.method.ts (5)
packages/render-helper/src/methods/a.method.ts (1)
a(58-801)packages/render-helper/src/methods/iframe.method.ts (1)
iframe(3-175)packages/render-helper/src/methods/text.method.ts (1)
text(7-105)packages/render-helper/src/methods/img.method.ts (1)
img(3-49)packages/render-helper/src/methods/p.method.ts (1)
p(1-6)
packages/render-helper/src/methods/iframe.method.ts (1)
packages/render-helper/src/consts/regexes.const.ts (20)
YOUTUBE_EMBED_REGEX(15-15)BITCHUTE_REGEX(18-18)VIMEO_EMBED_REGEX(17-17)TWITCH_EMBED_REGEX(38-38)SPEAK_EMBED_REGEX(30-30)SPOTIFY_EMBED_REGEX(36-36)SOUNDCLOUD_EMBED_REGEX(37-37)D_TUBE_EMBED_REGEX(21-21)VIMM_EMBED_REGEX(35-35)DAPPLR_REGEX(23-23)TRUVVL_REGEX(24-24)LBRY_REGEX(25-25)ODYSEE_REGEX(26-26)SKATEHIVE_IPFS_REGEX(27-27)ARCH_REGEX(28-28)RUMBLE_REGEX(33-33)BRIGHTEON_REGEX(34-34)BRAND_NEW_TUBE_REGEX(39-39)LOOM_EMBED_REGEX(41-41)AUREAL_EMBED_REGEX(42-42)
packages/render-helper/src/methods/markdown-to-html.method.ts (5)
packages/render-helper/src/methods/linkify.method.ts (1)
linkify(6-61)packages/render-helper/src/consts/regexes.const.ts (1)
ENTITY_REGEX(43-43)packages/render-helper/src/consts/dom-parser.const.ts (1)
DOMParser(17-21)packages/render-helper/src/methods/traverse.method.ts (1)
traverse(7-33)packages/render-helper/src/methods/sanitize-html.method.ts (1)
sanitizeHtml(9-33)
🔇 Additional comments (12)
packages/render-helper/src/methods/markdown-to-html.method.ts (3)
32-52: LGTM on Remarkable configuration.The Markdown processor setup with HTML enabled, proper highlight fallback handling, and plugin configuration is well-structured. The error logging on line 48 appropriately allows rendering to continue even if syntax highlighting fails.
67-71: LGTM on serializer setup and early return.The early return for empty input prevents unnecessary processing.
140-154: LGTM on output cleanup and final sanitization.The entity restoration correctly mirrors the encoding logic, and the final
sanitizeHtmlcall provides defense-in-depth against any XSS vectors that might slip through DOM manipulation.packages/render-helper/src/methods/iframe.method.ts (2)
59-67: SoundCloud iframe falls through to unsupported if URL extraction fails.When
SOUNDCLOUD_EMBED_REGEXmatches but the innerurl=(.+?)&pattern doesn't, the iframe is replaced with "unsupported". This may be intentional, but consider whether a partial match should preserve the original iframe or explicitly return early.
1-11: LGTM - Clean structure with proper early exits.The guard clauses handle null elements and missing sources appropriately, and the provider-specific branching is well-organized.
packages/render-helper/src/methods/a.method.ts (7)
76-80: Good XSS protection for javascript: URIs.The case-insensitive check for
javascript:hrefs properly prevents XSS attacks.
573-594: LGTM - Correct regex state management.The
lastIndex = 0reset beforeexec()calls prevents stale state issues with global regexes. This pattern is consistently applied.
82-100: LGTM - Image link transformation with proper document import.The
importNodecall ensures the node fromDOMParseris properly imported into the target document before replacement.
124-155: LGTM - Proper validation for Hive post links.The combination of whitelist domain checking,
sanitizePermlink(),isValidPermlink(), andisValidUsername()provides comprehensive input validation.
717-746: Good secure DOM construction for Twitter embeds.Using
textContentand programmatic DOM creation avoids XSS risks from potentially malicious URL/author values.
792-799: LGTM - Proper security attributes for external links.The
target="_blank"withrel="noopener"correctly prevents reverse tabnabbing attacks.
1-35: Well-organized imports and helper functions.The imports are grouped logically and the helper functions (
normalizeValue,matchesHref,getInlineMeta) are well-scoped utilities that reduce duplication.
There was a problem hiding this comment.
Actionable comments posted: 7
🤖 Fix all issues with AI agents
In @packages/render-helper/src/markdown-2-html.spec.ts:
- Line 210: The test description string in the spec for the case "16- Should
replace placeholder for unsopported iframe sources" contains a typo; update the
'it' description to read "16- Should replace placeholder for unsupported iframe
sources" so the wording is correct (look for the test case in render-helper's
markdown-2-html.spec.ts with the 'it' call containing "unsopported").
In @packages/render-helper/src/methods/a.method.ts:
- Around line 409-411: The CCC link check uses cccMatch[1] directly against
WHITE_LIST, so links with a leading "www." won't match; normalize the extracted
host before comparing by trimming a leading "www." (e.g., derive a
normalizedHost from href.match(CCC_REGEX) / cccMatch[1] that strips a starting
"www.") and then use that normalizedHost in the WHITE_LIST.includes(...) check
before calling el.setAttribute('class', 'markdown-post-link').
- Around line 200-207: The substring check using SECTION_LIST.some(v =>
tpostMatch[3].includes(v)) can produce false positives (e.g.,
"wallet-recovery-guide" matching "wallet"); update the logic in the
isValidDomain branch (where tpostMatch[3] is used, around the block that calls
el.setAttribute and isValidPermlink) to first isolate the path segment before
any query string (split on '?' and take [0]) and then check against SECTION_LIST
using exact equality or startsWith with a separator (e.g., segment === v ||
segment.startsWith(v + '/') as appropriate) instead of includes(); apply the
same fix to the similar pattern found at the later occurrence near line 287.
- Around line 382-384: The domain whitelist check using comMatch (result of
CUSTOM_COMMUNITY_REGEX) does not strip a leading "www." so domains like
"www.ecency.com" fail WHITE_LIST.includes; update the conditional to normalize
comMatch[1] by removing a leading "www." (e.g., replace(/^www\./i, '') or
similar) before checking WHITE_LIST.includes, then proceed to call
el.setAttribute('class', 'markdown-community-link') when the normalized domain
is in the whitelist.
In @packages/render-helper/src/methods/iframe.method.ts:
- Around line 141-146: The comment above the BrightEon handling block has a
typo: change the comment text "Brigtheon" to the correct "Brighteon" where the
BRIGHTEON_REGEX match is handled (around the block that sets
el.setAttribute('src', src) and 'frameborder', '0'); update the single-line
comment to the correct spelling to avoid confusion.
- Around line 60-68: The Soundcloud branch currently only handles URLs when the
inner extraction uses /url=(.+?)&/, so if SOUNDCLOUD_EMBED_REGEX matches but
that inner match fails the flow falls through; update the logic in
iframe.method.ts so that when SOUNDCLOUD_EMBED_REGEX matches you either (a) use
a more robust extraction like /url=(.+?)(?:&|$)/ to capture a trailing parameter
or end-of-string, set the iframe src via el.setAttribute('src', ...) and return,
or (b) if extraction still fails, perform an explicit fallback return (or set a
safe fallback src) to prevent falling through to subsequent provider checks.
Ensure the change touches the Soundcloud block around SOUNDCLOUD_EMBED_REGEX and
the match handling so a return always happens when the provider is recognized.
- Around line 33-39: The Twitch embed branch using TWITCH_EMBED_REGEX can append
duplicate query params; update the logic that builds s from src to first parse
or inspect src for existing "parent=" and "autoplay=" parameters (e.g., via URL
or regex) and only add missing ones, preserving the correct separator (& or ?)
based on whether other params exist, then set the cleaned/updated value on
el.setAttribute('src', ...); reference TWITCH_EMBED_REGEX, src, parentDomain,
and el.setAttribute('src', ...) when locating and updating the code.
🧹 Nitpick comments (9)
packages/render-helper/src/markdown-2-html.spec.ts (3)
1-5: Inconsistent module system usage.Lines 1-2 use ES module imports while lines 4-5 use CommonJS
require. For consistency and to align with modern TypeScript practices, consider using ES module imports throughout.🔧 Suggested fix
import { getTestData } from './test/data' import { markdown2Html } from './markdown-2-html' +import * as fs from 'fs' +import * as path from 'path' -const fs = require('fs') -const path = require('path')
6-14: Consider extracting large snapshot data to a separate file.The
SNAPSHOT_JSONobject contains substantial expected HTML outputs. For improved maintainability and readability, consider moving this to a dedicated JSON fixture file (e.g.,./test/data/snapshots.json).
1149-1178: Good use of data-driven testing for legacy files.The dynamic test generation pattern is well-suited for testing against multiple fixture files. A couple of minor suggestions:
- Consider filtering by
.jsonextension rather than excluding.DS_Storefor robustness against other hidden/system files.- The magic number
150on line 1154 could use a brief comment explaining its purpose (avoiding ID collisions with other tests).🔧 More robust file filtering
- for (const file of files) { - if (file === '.DS_Store') { - continue - } + for (const file of files.filter(f => f.endsWith('.json'))) {packages/render-helper/src/methods/iframe.method.ts (1)
70-167: Consider a provider configuration map for maintainability.The repeated pattern of
setAttributecalls could be consolidated using a configuration object mapping each regex to its required attributes. This would reduce duplication and make adding new providers easier.Example approach
interface ProviderConfig { regex: RegExp; sandbox?: string; frameborder?: string; allowfullscreen?: string; className?: string; } const SIMPLE_PROVIDERS: ProviderConfig[] = [ { regex: D_TUBE_EMBED_REGEX, sandbox: 'allow-scripts allow-same-origin', frameborder: '0', allowfullscreen: 'true' }, { regex: VIMM_EMBED_REGEX, sandbox: 'allow-scripts allow-same-origin allow-popups', frameborder: '0', allowfullscreen: 'true' }, // ... other providers ]; // Then iterate: for (const provider of SIMPLE_PROVIDERS) { if (src.match(provider.regex)) { if (provider.sandbox) el.setAttribute('sandbox', provider.sandbox); if (provider.frameborder) el.setAttribute('frameborder', provider.frameborder); if (provider.allowfullscreen) el.setAttribute('allowfullscreen', provider.allowfullscreen); if (provider.className) el.setAttribute('class', provider.className); return; } }packages/render-helper/src/methods/a.method.ts (5)
47-55: Unused variable and redundant property.
titleMatchesis computed on line 49 but never used. Additionally,isInlineis always identical totextMatches, making it redundant.♻️ Suggested simplification
const getInlineMeta = (el: HTMLElement, href: string) => { const textMatches = matchesHref(href, el.textContent) - const titleMatches = matchesHref(href, el.getAttribute('title')) return { - textMatches, - isInline: textMatches + isInline: textMatches } }
796-799: Consider addingnoreferrerfor enhanced privacy.Adding
noreferreralongsidenoopenerprevents sending theRefererheader to external sites, which is a common privacy best practice.♻️ Suggested enhancement
} else { el.setAttribute('target', '_blank'); - el.setAttribute('rel', 'noopener'); + el.setAttribute('rel', 'noopener noreferrer'); }
58-58: Unused parameterparentDomain.The
parentDomainparameter is declared with a default value but never used in the function body. Consider removing it if not needed, or adding a TODO comment if it's planned for future use.
534-538: Consider consistent iframe sandboxing.Spotify and Loom iframes include
sandboxattributes, but Vimeo and Twitch iframes do not. For consistency, consider applying sandboxing to all third-party embeds, though you may need to test that Vimeo/Twitch function correctly with restrictions likeallow-scripts allow-same-origin allow-popups.
58-801: Consider decomposing into smaller handler functions.The
a()function handles many URL patterns (~750 lines). While each section uses early returns effectively, extracting handlers for major categories (e.g.,handleHivePost,handleVideoEmbed,handleSocialEmbed) would improve readability and testability.This could be deferred as the current structure is functional.
📜 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 (4)
packages/render-helper/src/markdown-2-html.spec.tspackages/render-helper/src/methods/a.method.tspackages/render-helper/src/methods/iframe.method.tspackages/render-helper/src/methods/traverse.method.ts
🚧 Files skipped from review as they are similar to previous changes (1)
- packages/render-helper/src/methods/traverse.method.ts
🧰 Additional context used
🧬 Code graph analysis (2)
packages/render-helper/src/methods/a.method.ts (9)
packages/render-helper/dist/browser/index.js (30)
isLCP(345-345)isLCP(409-409)isLCP(1161-1161)tag(434-434)tag(505-505)permlink(436-436)permlink(512-512)permlink(575-575)permlink(660-660)domain(483-483)section(491-491)section(559-559)ha(497-497)ha(564-564)community(636-636)vid(683-683)vid(696-696)vid(709-709)vid(724-724)match(720-720)match(993-993)src(331-331)src(818-818)src(863-863)src(954-954)src(1418-1418)e(884-884)e(1171-1171)m(911-911)m(967-967)packages/render-helper/src/consts/regexes.const.ts (11)
IMG_REGEX(3-3)IPFS_REGEX(4-4)POST_REGEX(5-5)MENTION_REGEX(7-7)INTERNAL_POST_TAG_REGEX(11-11)INTERNAL_MENTION_REGEX(9-9)INTERNAL_POST_REGEX(12-12)TOPIC_REGEX(8-8)INTERNAL_TOPIC_REGEX(10-10)CUSTOM_COMMUNITY_REGEX(13-13)CCC_REGEX(6-6)packages/render-helper/src/methods/get-inner-html.method.ts (1)
getSerializedInnerHTML(3-11)packages/render-helper/src/methods/img.method.ts (2)
createImageHTML(51-62)img(3-49)packages/render-helper/src/consts/dom-parser.const.ts (1)
DOMParser(17-21)packages/render-helper/src/methods/remove-child-nodes.method.ts (1)
removeChildNodes(1-6)packages/render-helper/src/consts/white-list.const.ts (1)
WHITE_LIST(1-18)packages/render-helper/src/helper.ts (2)
sanitizePermlink(33-42)isValidPermlink(44-56)packages/render-helper/src/consts/section-list.const.ts (1)
SECTION_LIST(1-20)
packages/render-helper/src/methods/iframe.method.ts (1)
packages/render-helper/src/consts/regexes.const.ts (14)
YOUTUBE_EMBED_REGEX(15-15)BITCHUTE_REGEX(18-18)VIMEO_EMBED_REGEX(17-17)DAPPLR_REGEX(23-23)TRUVVL_REGEX(24-24)LBRY_REGEX(25-25)ODYSEE_REGEX(26-26)SKATEHIVE_IPFS_REGEX(27-27)ARCH_REGEX(28-28)RUMBLE_REGEX(33-33)BRIGHTEON_REGEX(34-34)BRAND_NEW_TUBE_REGEX(39-39)LOOM_EMBED_REGEX(41-41)AUREAL_EMBED_REGEX(42-42)
🔇 Additional comments (14)
packages/render-helper/src/markdown-2-html.spec.ts (3)
1034-1034: Verify "Brightreon" spelling.The describe block uses "Brightreon" but the video platform is typically spelled "Brighteon". If this is intentional (matching how the platform refers to itself), disregard this comment.
1085-1133: Good sanitization test coverage.The sanitization tests appropriately cover critical XSS vectors including JavaScript links, script tag injection, and malicious event handlers. This is essential for a markdown-to-HTML converter.
16-1251: Comprehensive test suite for markdown-to-HTML conversion.The test file provides extensive coverage including:
- Link transformations (internal, external, post links, author mentions, tags)
- Video embeds (YouTube, Vimeo, DTube, Twitch, Rumble, etc.)
- Iframe handling and sanitization
- Security measures (XSS prevention)
- Data-driven legacy tests
- WebP image format support
The structure is well-organized with logical groupings. The tests will help ensure the render-helper package maintains correctness across various input scenarios.
packages/render-helper/src/methods/iframe.method.ts (6)
3-11: LGTM!Good defensive programming with null checks for the element, parent node, and src attribute. The early bailout and element removal for missing src is appropriate.
13-23: LGTM!YouTube query string stripping and Bitchute pass-through are straightforward and correct.
41-50: Verify: autoplay defaults totruefor 3Speak vsfalsefor Twitch.3Speak defaults to
autoplay=true(line 47) while Twitch defaults toautoplay=false(line 36). If this is intentional for different provider behaviors, consider adding a brief comment explaining the rationale.
70-105: LGTM!Provider handling for Dtube, VIMM, Dapplr, and Truvvl follows a consistent pattern with appropriate sandbox policies.
107-132: LGTM!LBRY.tv, Odysee, Skatehive IPFS, and archive.org handling is straightforward with minimal necessary attributes.
169-176: LGTM!The fallback correctly uses
textContent(notinnerHTML) to safely display the unsupported src without XSS risk.packages/render-helper/src/methods/a.method.ts (5)
76-80: Good XSS protection forjavascript:URIs.The case-insensitive check and removal of
javascript:hrefs is a proper security measure.
117-119: IPFS image src not proxified.Unlike other image handling (e.g., line 90, 496, 637, 698), IPFS image URLs are set directly without
proxifyImageSrc. If IPFS images should bypass the proxy intentionally, consider adding a comment. Otherwise, apply consistent proxification.♻️ If proxification is intended
const img = el.ownerDocument.createElement('img') - img.setAttribute('src', href) + img.setAttribute('src', proxifyImageSrc(href, 0, 0, webp ? 'webp' : 'match')) el.appendChild(img)
726-743: Good XSS protection for Twitter embeds.Using DOM construction methods (
createElement,textContent,setAttribute) instead of string interpolation properly escapes user-controlledurlandauthorvalues, preventing XSS.
755-768: Proper error handling for malformed URIs.The try-catch around
decodeURIcorrectly handles potentially malformed input, allowing the function to continue processing instead of throwing.
574-576: Good practice: resetting regex lastIndex.Resetting
lastIndexbeforeexec()on global regexes prevents state leakage between calls. This pattern is correctly applied forSPOTIFY_REGEX,D_TUBE_REGEX,D_TUBE_REGEX2, andTWITTER_REGEX.
There was a problem hiding this comment.
Actionable comments posted: 3
🤖 Fix all issues with AI agents
In @packages/render-helper/src/markdown-2-html.spec.ts:
- Line 1034: The describe block title is misspelled as "Brightreon support";
update the describe call string to "Brighteon support" so it matches the
platform name used in the individual tests (e.g., the test names referencing
"Brighteon")—locate the describe("Brightreon support", ...) and change the
string to "Brighteon support".
In @packages/render-helper/src/methods/a.method.ts:
- Around line 797-805: The code prepends "https://" to href in one branch but
never updates the DOM element in the web branch, so the element keeps the
original scheme-less href; update the element's href after you normalize/modify
the href value (ensure the variable href is reassigned with the prepended URL)
and call el.setAttribute('href', href) in both branches (or once after
branching) so the normalized URL is applied for web mode; adjust the logic
around SECTION_REGEX, el, and the existing forApp branch so the modified href is
used consistently.
In @packages/render-helper/src/methods/iframe.method.ts:
- Around line 13-176: The provider regexes (e.g., VIMEO_EMBED_REGEX,
SPOTIFY_EMBED_REGEX, SOUNDCLOUD_EMBED_REGEX, VIMM_EMBED_REGEX,
TWITCH_EMBED_REGEX, D_TUBE_EMBED_REGEX, LBRY_REGEX, ODYSEE_REGEX, ARCH_REGEX,
etc.) lack end anchors, allowing URLs with extra path/host segments to bypass
validation; update each pattern in consts/regexes.const.ts to include proper end
anchors (and where appropriate start anchors) so they match the full expected
URL (e.g., append `$` or use `(?:$|[?#])` as needed), then run the iframe logic
(functions/methods referencing these constants such as the iframe handling
routine in iframe.method.ts) to ensure behavior is unchanged; optionally
refactor the long if/return chain into a provider registry (mapping regex →
config/sandbox/attrs and a single loop) after tests pass.
🧹 Nitpick comments (7)
packages/render-helper/src/markdown-2-html.spec.ts (3)
4-5: Mixed module syntax: prefer consistent ES module imports.The file uses ES module imports on lines 1-2 but switches to CommonJS
requireforfsandpath. For consistency and modern TypeScript conventions, consider using ES module imports throughout.Suggested fix
import { getTestData } from './test/data' import { markdown2Html } from './markdown-2-html' +import * as fs from 'fs' +import * as path from 'path' -const fs = require('fs') -const path = require('path')
6-14: Large inline snapshot data could be externalized.The
SNAPSHOT_JSONobject contains lengthy HTML strings that reduce readability. Consider moving these to a separate JSON file (e.g.,snaps.jsonmentioned in the AI summary) or using Jest's built-in snapshot testing with.toMatchSnapshot().
1149-1178: Dynamic test generation uses synchronous file operations and magic number.A few observations on this section:
- The starting value
150forxis a magic number - consider extracting as a named constant or using a more descriptive approach.- The
.DS_Storecheck (line 1157) is macOS-specific. Consider filtering by file extension instead.- Synchronous
fs.readdirSyncandfs.readFileSyncare acceptable in test setup, but could impact test startup time with many files.Suggested improvements
describe('Test legacy files', () => { const dataDir = `${__dirname}/test/data/legacy` - const files = fs.readdirSync(dataDir) - - let x = 150 - - for (const file of files) { - if (file === '.DS_Store') { - continue - } + const jsonFiles = files.filter(file => file.endsWith('.json')) + + jsonFiles.forEach((file, index) => { const fileContents = fs.readFileSync(path.join(dataDir, file), 'utf8') const data = JSON.parse(fileContents) const id = data['id'] const input = { - author: 'foo' + x, - permlink: 'bar' + x, + author: `legacy-author-${index}`, + permlink: `legacy-permlink-${index}`, last_update: '2019-05-10T09:15:21', body: data['input'] } const expected = data['result'] it('ID: ' + id, function () { expect(markdown2Html(input)).toBe(expected) }) - - x += 1 - } + }) })packages/render-helper/src/methods/iframe.method.ts (1)
61-67: RedundantsetAttribute('src', src)call.Line 63 sets
srcto the value already present on the element. This pattern repeats for DTube (81), VIMM (90), Dapplr (100), Truvvl (108), LBRY (118), Odysee (125), IPFS Skatehive (132), archive.org (139), Rumble (145), Brighteon (151), BrandNew Tube (159), Loom (166), and Aureal (173).Consider removing redundant calls to reduce noise:
Example fix for Spotify
// Spotify if (src.match(SPOTIFY_EMBED_REGEX)) { - el.setAttribute('src', src); el.setAttribute('sandbox', 'allow-scripts allow-same-origin allow-popups'); el.setAttribute('frameborder', '0'); return; }packages/render-helper/src/methods/a.method.ts (3)
538-542: Inconsistent iframe sandboxing across video embeds.Spotify (line 594) and Loom (line 615) iframes have
sandboxattributes, but Vimeo and Twitch iframes don't. Consider applying consistent sandboxing to all third-party embeds to limit their capabilities.🔒 Suggested sandbox attributes for Vimeo
const ifr = el.ownerDocument.createElement('iframe') ifr.setAttribute('frameborder', '0') ifr.setAttribute('allowfullscreen', 'true') ifr.setAttribute('src', embedSrc) + ifr.setAttribute('sandbox', 'allow-scripts allow-same-origin allow-popups') el.appendChild(ifr)
47-55: Unused variabletitleMatchescan be removed.
titleMatchesis computed but never used in the return value or elsewhere. Consider removing it to reduce confusion.♻️ Suggested cleanup
const getInlineMeta = (el: HTMLElement, href: string) => { const textMatches = matchesHref(href, el.textContent) - const titleMatches = matchesHref(href, el.getAttribute('title')) return { textMatches, isInline: textMatches } }
126-127: Consider extracting repeated domain normalization logic.The pattern
.replace(/^www\./, '')appears multiple times (lines 126, 159, 343, 387, 414). A helper function would improve consistency and reduce duplication.♻️ Example helper
const stripWwwPrefix = (domain: string): string => domain.replace(/^www\./, ''); // Usage: if (postMatch && WHITE_LIST.includes(stripWwwPrefix(postMatch[1]))) {
📜 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.tspackages/render-helper/src/methods/iframe.method.ts
🔇 Additional comments (9)
packages/render-helper/src/markdown-2-html.spec.ts (3)
16-989: Comprehensive test coverage for markdown traversing scenarios.The "Traversing" test suite provides excellent coverage across 70 test cases including:
- Image handling and proxification
- Various video platform embeds (YouTube, Vimeo, Twitch, DTube, 3Speak, etc.)
- Internal/external link handling
- Author mentions and tag linking
- Iframe sanitization and embed handling
The tests are well-structured with clear input/expected output patterns.
1085-1133: Security sanitization tests are well-designed.The sanitization tests properly cover critical security scenarios:
- JavaScript link removal (XSS prevention)
- Script tag conversion
- Removal of dangerous attributes like
onclickThese tests ensure the converter properly sanitizes user-generated content.
1229-1242: Reply cleaning test validates removal of promotional footers.The
cleanReplytest verifies that common promotional footers (Partiko, Dapplr, LeoFinance, STEMGeeks, Aeneas.Blog, weedcash.network) are stripped from reply bodies. This is good for maintaining clean content display.packages/render-helper/src/methods/iframe.method.ts (4)
3-11: LGTM!The function signature with null-safe parameter and default domain is well-designed. The early validation correctly handles edge cases: null elements, missing parent nodes, and iframes without
srcattributes.
50-59: Inconsistent autoplay default for 3Speak.3Speak defaults to
autoplay=true(line 56), while Twitch (line 44) and SoundCloud (line 73) default toautoplay=false. This inconsistency could lead to unexpected behavior. Modern browsers often block autoplaying media anyway.Was this intentional, or should it align with other providers using
autoplay=false?Suggested fix to align with other providers
const hasAutoplay = /[?&]autoplay=/.test(normalizedSrc); const s = hasAutoplay ? normalizedSrc - : `${normalizedSrc}${normalizedSrc.includes('?') ? '&' : '?'}autoplay=true`; + : `${normalizedSrc}${normalizedSrc.includes('?') ? '&' : '?'}autoplay=false`; el.setAttribute('src', s);
69-77: LGTM!The SoundCloud handling correctly extracts the URL parameter and rebuilds the player URL. The non-greedy regex with
(?:&|$)properly handles both cases. Good defensive coding with the explicit return on line 76 regardless of match success.
178-184: LGTM - Safe fallback implementation.Using
textContentcorrectly prevents XSS when displaying the unsupported URL. The parentNode null check at line 181 is slightly redundant (already checked at line 4) but provides good defensive coding in case this function is refactored later.packages/render-helper/src/methods/a.method.ts (2)
798-800: Remove this review comment.
SECTION_REGEXis defined without the global flag (/\B(\#[\da-zA-Z-_]+\b)(?!;)/i), solastIndexreset is not applicable. The.match()method is the correct choice here. Other regexes mentioned (SPOTIFY_REGEX, D_TUBE_REGEX, TWITTER_REGEX) have the global flag and use.exec(), which requireslastIndexresets—a different scenario entirely.Likely an incorrect or invalid review comment.
117-119: Proxify IPFS image source for consistency with other image handlers.Regular images use
createImageHTML()which appliesproxifyImageSrc()(img.method.ts:54), and video thumbnails explicitly callproxifyImageSrc(). However, IPFS images here are added directly withsetAttribute('src', href)without proxification. This creates inconsistent handling that could affect caching and security behavior across different image types.
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Fix all issues with AI agents
In @packages/render-helper/src/consts/regexes.const.ts:
- Around line 19-20: The regex D_TUBE_REGEX is using an unescaped dot in
"d.tube.#!", which lets "." match any character and can incorrectly match
strings like "d.tubex#!"; update the pattern to match the literal dots and slash
by escaping them (e.g., change D_TUBE_REGEX to use "d\.tube\/#!\/" or equivalent
so the dot in "d.tube" is escaped and the character before "#!" is a literal "/"
rather than "."). Ensure the rest of the groups (\w+)\/(\w+) remain unchanged.
🧹 Nitpick comments (3)
packages/render-helper/src/methods/a.method.ts (3)
127-127: Consider extracting domain whitelist validation into a helper.The pattern
WHITE_LIST.includes(match[1].replace(/^www\./,''))is repeated at lines 127, 160, 344, 388, and 415. A helper function would reduce duplication and centralize the validation logic.Example helper extraction
const isWhitelistedDomain = (domain: string): boolean => WHITE_LIST.includes(domain.replace(/^www\./, ''))Then use:
isWhitelistedDomain(postMatch[1])
539-546: Consider adding sandbox attribute to Vimeo and Twitch iframes for consistency.Spotify (line 595) and Loom (line 616) iframes include sandbox attributes for security, but Vimeo and Twitch iframes don't. Consider adding consistent sandboxing across all embedded iframes.
Suggested addition
const ifr = el.ownerDocument.createElement('iframe') ifr.setAttribute('frameborder', '0') ifr.setAttribute('allowfullscreen', 'true') ifr.setAttribute('src', embedSrc) + ifr.setAttribute('sandbox', 'allow-scripts allow-same-origin allow-popups') el.appendChild(ifr)
802-805: Consider addingnoreferreralongsidenoopenerfor external links.While
noopenerprevents the opened page from accessingwindow.opener, addingnoreferreralso prevents theRefererheader from being sent, providing additional privacy for users clicking external links.el.setAttribute('target', '_blank'); - el.setAttribute('rel', 'noopener'); + el.setAttribute('rel', 'noopener noreferrer');
📜 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/consts/regexes.const.tspackages/render-helper/src/markdown-2-html.spec.tspackages/render-helper/src/methods/a.method.ts
🚧 Files skipped from review as they are similar to previous changes (1)
- packages/render-helper/src/markdown-2-html.spec.ts
🔇 Additional comments (6)
packages/render-helper/src/consts/regexes.const.ts (1)
1-45: Comprehensive regex collection for link parsing.The regex patterns cover a wide range of platforms and URL formats. The structure is clean with clear naming conventions. The patterns correctly handle various video/audio embed formats.
packages/render-helper/src/methods/a.method.ts (5)
36-56: Clean helper function implementations.The helper functions
normalizeValue,matchesHref, andgetInlineMetaare well-designed with clear single responsibilities.
77-81: Good XSS prevention forjavascript:protocol.The check is case-insensitive and handles whitespace, which properly blocks XSS attempts like
JaVaScRiPt:alert(1).
83-101: LGTM - Image URL handling with proper DOM manipulation.The code correctly handles the case where an anchor's href and content are both image URLs, replacing the anchor with an actual image element using safe DOM methods.
723-752: Secure Twitter embed handling with proper XSS prevention.Good use of DOM construction methods (
createElement,textContent,setAttribute) that automatically escape content, rather than string concatenation with innerHTML.
59-808: Well-structured anchor transformation function with comprehensive link handling.The function handles a wide variety of link types with proper:
- Null guards and early returns
- XSS prevention (javascript: blocking, DOM construction instead of innerHTML)
- Distinction between app and web rendering modes
- Regex state management (lastIndex resets)
The sequential pattern matching with early returns keeps the logic readable despite the function's length.
Summary by CodeRabbit
New Features
Tests
Chores
Documentation
Bug Fix / UX
✏️ Tip: You can customize this high-level summary in your review settings.