Skip to content
Draft
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
69 changes: 69 additions & 0 deletions src/components/Item.scss
Original file line number Diff line number Diff line change
@@ -0,0 +1,69 @@
@use "../app/shared/scss/media" as *;

p {
margin: 2px 0;

@media #{$mobile-only} {
margin-bottom: 5px;
margin-top: 0;
}
}

a {
cursor: pointer;
text-decoration: none;
}

.title {
font-size: 16px;
font-family: Verdana, Geneva, sans-serif;
}

.subtext-laptop {
font-size: 12px;
font-weight: bold;
letter-spacing: 0.5px;

a {
&:hover {
text-decoration: underline;
}
}

@media #{$mobile-only} {
display: none;
}
}

.subtext-palm {
font-size: 13px;
font-weight: bold;
letter-spacing: 0.5px;

a {
&:hover {
text-decoration: underline;
}
}

.details {
margin-top: 5px;

.right {
float: right;
}
}

@media #{$laptop-only} {
display: none;
}
}

.domain {
color: #696969;
letter-spacing: 0.5px;
}

.item-details {
padding: 10px;
}
83 changes: 83 additions & 0 deletions src/components/Item.tsx
Original file line number Diff line number Diff line change
@@ -0,0 +1,83 @@
import { Link } from 'react-router-dom';
import type { Story } from '../types/Story';
import { useSettings } from '../hooks/useSettings';
import { formatCommentCount } from '../utils/formatCommentCount';
import './Item.scss';

interface ItemProps {
item: Story;
index: number;
}

export function Item({ item }: ItemProps) {
const { settings } = useSettings();
const hasUrl = item.url?.indexOf('http') === 0;

return (
<div style={{ marginBottom: `${settings.listSpacing}px` }}>
{hasUrl ? (
<p>
<a
className="title"
style={{ fontSize: `${settings.titleFontSize}px` }}
href={item.url}
target={settings.openLinkInNewTab ? '_blank' : undefined}
rel={settings.openLinkInNewTab ? 'noopener' : undefined}
>
{item.title}
</a>
{item.domain && <span className="domain">({item.domain})</span>}
</p>
) : (
<p>
<Link
className="title"
style={{ fontSize: `${settings.titleFontSize}px` }}
to={`/item/${item.id}`}
>
{item.title}
</Link>
</p>
)}

<div className="subtext-palm">
{item.type !== 'job' && (
<div className="details">
<span className="name">
<Link to={`/user/${item.user}`}>{item.user}</Link>
</span>
<span className="right">{item.points} &#9733;</span>
</div>
)}
<div className="details">
{item.time_ago}
{item.type !== 'job' && (
<Link to={`/item/${item.id}`} className="comment-number">
{' '}&bull; {formatCommentCount(item.comments_count)}
</Link>
)}
</div>
</div>

<div className="subtext-laptop">
{item.type !== 'job' && (
<span>
{item.points} points by{' '}
<Link to={`/user/${item.user}`}>{item.user}</Link>
</span>
)}
<span className={item.type !== 'job' ? 'item-details' : undefined}>
{item.time_ago}
{item.type !== 'job' && (
<span>
{' '}|{' '}
<Link to={`/item/${item.id}`}>
{formatCommentCount(item.comments_count)}
</Link>
</span>
)}
</span>
</div>
</div>
);
}
104 changes: 104 additions & 0 deletions src/pages/Feed.scss
Original file line number Diff line number Diff line change
@@ -0,0 +1,104 @@
@use "../app/shared/scss/media" as *;

a {
text-decoration: none;
font-weight: bold;

&:hover {
text-decoration: underline;
}
}

ol {
padding: 0 40px;
margin: 0;

@media #{$mobile-only} {
box-sizing: border-box;
list-style: none;
padding: 0 10px;
}

li {
position: relative;
transition: background-color 0.2s ease;
}
}

.list-margin {
@media #{$mobile-only} {
margin-top: 55px;
}
}

.main-content {
position: relative;
width: 100%;
min-height: 100vh;
transition: opacity 0.2s ease;
box-sizing: border-box;
padding: 8px 0;
z-index: 0;
}

.post {
padding: 10px 0 10px 5px;
transition: background-color 0.2s ease;
border-bottom: 1px solid #cececb;

.itemNum {
color: #696969;
position: absolute;
width: 30px;
text-align: right;
left: 0;
top: 4px;
}
}

.item-block {
display: block;
}

.nav {
padding: 10px 40px;
margin-top: 10px;
font-size: 17px;

a {
@media #{$mobile-only} {
text-decoration: none;
}
}

@media #{$mobile-only} {
margin: 20px 0;
text-align: center;
padding: 10px 80px;
height: 20px;
}

.prev {
padding-right: 20px;

@media #{$mobile-only} {
float: left;
padding-right: 0;
}
}

.more {
@media #{$mobile-only} {
float: right;
}
}
}

.job-header {
font-size: 15px;
padding: 0 40px 10px;

@media #{$mobile-only} {
padding: 60px 15px 25px 15px;
}
}
113 changes: 113 additions & 0 deletions src/pages/Feed.tsx
Original file line number Diff line number Diff line change
@@ -0,0 +1,113 @@
import { useEffect, useReducer } from 'react';
import { useParams, Link } from 'react-router-dom';
import { fetchFeed } from '../api/hackerNewsApi';
import type { Story } from '../types/Story';
import { Item } from '../components/Item';
import { Loader } from '../components/Loader';
import { ErrorMessage } from '../components/ErrorMessage';
import './Feed.scss';

interface FeedProps {
feedType: string;
}

interface FeedState {
items: Story[] | null;
errorMessage: string;
}

type FeedAction =
| { type: 'loading' }
| { type: 'loaded'; items: Story[] }
| { type: 'error'; message: string };

function feedReducer(_state: FeedState, action: FeedAction): FeedState {
switch (action.type) {
case 'loading':
return { items: null, errorMessage: '' };
case 'loaded':
return { items: action.items, errorMessage: '' };
case 'error':
return { items: null, errorMessage: action.message };
}
}

export function Feed({ feedType }: FeedProps) {
const { page } = useParams<{ page: string }>();
const pageNum = page ? parseInt(page, 10) : 1;
const [state, dispatch] = useReducer(feedReducer, {
items: null,
errorMessage: '',
});

useEffect(() => {
let cancelled = false;
dispatch({ type: 'loading' });

fetchFeed(feedType, pageNum)
.then((data) => {
if (!cancelled) {
dispatch({ type: 'loaded', items: data });
window.scrollTo(0, 0);
}
})
.catch(() => {
if (!cancelled) {
dispatch({
type: 'error',
message: `Could not load ${feedType} stories.`,
});
}
});

return () => { cancelled = true; };
}, [feedType, pageNum]);
Comment on lines +43 to +64
Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🟡 Race condition: stale fetch response overwrites current data when feedType or page changes rapidly

The useEffect in Feed.tsx:43-57 fires a new fetchFeed call whenever feedType or pageNum changes, but never cancels the previous in-flight request. If a user navigates quickly (e.g., page 1 → page 2 → page 3), multiple fetches run concurrently. If an earlier fetch resolves after a later one, its stale data overwrites the current results via dispatch({ type: 'loaded', items: data }), causing the UI to display data from a page/feed the user has already navigated away from. The fix is to use an AbortController or a stale-closure guard (boolean flag set in the cleanup function) to ignore responses from superseded requests.

Example scenario
  1. User on page 1 clicks "More" → effect fires fetch for page 2
  2. User immediately clicks "More" again → effect fires fetch for page 3, dispatches 'loading'
  3. Page 3 fetch completes first → UI shows page 3 data
  4. Page 2 fetch completes later → UI now incorrectly shows page 2 data while URL says page 3
Suggested change
useEffect(() => {
dispatch({ type: 'loading' });
fetchFeed(feedType, pageNum)
.then((data) => {
dispatch({ type: 'loaded', items: data });
window.scrollTo(0, 0);
})
.catch(() => {
dispatch({
type: 'error',
message: `Could not load ${feedType} stories.`,
});
});
}, [feedType, pageNum]);
useEffect(() => {
let cancelled = false;
dispatch({ type: 'loading' });
fetchFeed(feedType, pageNum)
.then((data) => {
if (!cancelled) {
dispatch({ type: 'loaded', items: data });
window.scrollTo(0, 0);
}
})
.catch(() => {
if (!cancelled) {
dispatch({
type: 'error',
message: `Could not load ${feedType} stories.`,
});
}
});
return () => { cancelled = true; };
}, [feedType, pageNum]);
Open in Devin Review

Was this helpful? React with 👍 or 👎 to provide feedback.

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Good catch — added a stale-closure guard (cancelled flag) in the cleanup function to ignore responses from superseded requests. Fixed in 3073209.


const { items, errorMessage } = state;
const listStart = (pageNum - 1) * 30 + 1;

return (
<div className="main-content">
{!items && !errorMessage && <Loader />}
{!items && errorMessage && <ErrorMessage message={errorMessage} />}

{items && (
<div>
{feedType === 'jobs' && (
<p className="job-header">
These are jobs at startups that were funded by Y Combinator. You
can also get a job at a YC startup through{' '}
<a href="https://triplebyte.com/?ref=yc_jobs">Triplebyte</a>.
</p>
)}

{feedType !== 'new' && (
<ol
className={feedType !== 'jobs' ? 'list-margin' : undefined}
start={listStart}
>
{items.map((item, index) => (
<li key={item.id} className="post">
<Item item={item} index={listStart + index} />
</li>
))}
</ol>
)}

<div className="nav">
{listStart !== 1 && (
<Link to={`/${feedType}/${pageNum - 1}`} className="prev">
&lsaquo; Prev
</Link>
)}
{items.length === 30 && (
<Link to={`/${feedType}/${pageNum + 1}`} className="more">
More &rsaquo;
</Link>
)}
</div>
</div>
)}
</div>
);
}