Item details and comment tree (migration 5/8) - #693
Item details and comment tree (migration 5/8)#693charityquinn-cognition wants to merge 4 commits into
Conversation
🤖 Devin AI EngineerI'll be helping with this pull request! Here's what you should know: ✅ I will automatically:
Note: I can only respond to comments from users who have write access to this repository. ⚙️ Control Options:
|
| useEffect(() => { | ||
| let ignore = false; | ||
| window.scrollTo(0, 0); | ||
| setErrorMessage(''); | ||
|
|
||
| fetchItemContent(itemID) | ||
| .then((nextItem) => { | ||
| if (!ignore) { | ||
| setItem(nextItem); | ||
| } | ||
| }) | ||
| .catch(() => { | ||
| if (!ignore) { | ||
| setErrorMessage('Could not load item comments.'); | ||
| } | ||
| }); | ||
|
|
||
| return () => { | ||
| ignore = true; | ||
| }; | ||
| }, [itemID]); |
There was a problem hiding this comment.
📝 Info: Stale item shown during item-to-item navigation
The effect resets errorMessage but not item before fetching, so navigating between items keeps the previous item's content and comments visible with no loader until the new fetch resolves. This matches the original Angular behavior, so it is not a regression.
Was this helpful? React with 👍 or 👎 to provide feedback.
Co-Authored-By: Charity Quinn <charity.quinn@cognition.ai>
Co-Authored-By: Charity Quinn <charity.quinn@cognition.ai>
Co-Authored-By: Charity Quinn <charity.quinn@cognition.ai>
895e94c to
6e369ac
Compare
Co-Authored-By: Charity Quinn <charity.quinn@cognition.ai>
| "@angular/platform-browser-dynamic": "~9.0.1", | ||
| "@angular/router": "~9.0.1", | ||
| "@angular/service-worker": "~9.0.1", | ||
| "dompurify": "3.4.13", |
There was a problem hiding this comment.
🔍 dompurify pin differs from description
package.json pins dompurify to 3.4.13 while the PR description says 3.2.6. Both are exact pins so builds stay deterministic; the mismatch is only between code and description. Confirm the intended version.
Was this helpful? React with 👍 or 👎 to provide feedback.
There was a problem hiding this comment.
3.4.13 is the intended version — the description was written against the original 3.2.6 pin and has since been updated. 3.2.6 carries 20 open Snyk findings (16 medium, 4 low: XSS, prototype pollution), which is a poor place to be for a sanitizer that ships in the browser bundle; 3.4.13 clears all of them within the same major and is confirmed gone from a rescan.
Summary
Stacked on #692. Ports
item-detailsand the recursivecommentcomponent, and adds/item/:id.fetchItemContent(sameignore-flag staleness guard as the feed), with the original loader/error states and messageCould not load item comments.contentHTML.Location.back()→navigate(-1).Commentrecurses throughcomment.comments, keeping collapse/expand as thehiddenattribute (so collapsed subtrees stay mounted, matching Angular) and keeping the deleted-comment markup. Child arrays are guarded (comment.comments ?? []) because the API omits the key on leaves — anditem.commentsis guarded the same way, since*ngForover a missing array rendered nothing where.mapwould throw.item.content,comment.content, poll option text) goes through a newsanitizeHtml()wrapper over DOMPurify:This is what keeps the port faithful rather than a hardening extra: Angular's
[innerHTML]runs Angular's DOM sanitizer, so the original stripped scripts and event handlers out of this API-provided markup. BaredangerouslySetInnerHTMLdoes not, which would have made user-authored comment/poll HTML executable in a visitor's browser. Adds one runtime dependency,dompurify, pinned to3.4.13— 3.2.6 carries 20 open Snyk findings (16 medium, 4 low: XSS, prototype pollution) and this is a sanitizer shipped in the browser bundle, so it's worth being current on.comment.component.scssused Angular's:host >>>piercing selector to style links inside that injected HTML. Outside view encapsulation:hostmatches nothing, so it's rescoped to.comment-text a— otherwise comment links would silently lose their bold/underline styling.Verified:
yarn react:buildpasses; nested comment trees render, collapse/expand works at depth, back returns to the originating feed page, and Ask HN text content renders. Sanitization leaves rich comment markup (links, paragraphs) intact.Devin-Org: engineering
Link to Devin session: https://app.devin.ai/sessions/1ff25c6cf2f949458f82cc596fc79c65
Requested by: @charityquinn-cognition
Devin Review