Refactor: Remove react-timeago
#7943
Conversation
|
The latest updates on your projects. Learn more about Vercel for Git ↗︎
1 Skipped Deployment
|
Dependency Review✅ No vulnerabilities or license issues or OpenSSF Scorecard issues found.OpenSSF Scorecard
Scanned Manifest Files |
There was a problem hiding this comment.
This is a cool change 😄 I'm very in favor of this overall, but have a few questions:
-
Does this look exactly the same as before? There are no screenies attached, so I'm guessing it's a drop-in replacement visually?
-
In what cases do we not want the component to update?
Question 2 is also pertinent to my inline comment regarding the props for that component. I'm not entirely convinced that the live?: number | boolean prop is the right way to format the API. But let's talk about it 💁🏼
| const TimeAgo: FC<TimeAgoProps> = ({ | ||
| date, | ||
| fallback = '', | ||
| refresh = true, |
There was a problem hiding this comment.
same point as before: it's weird to have an optional boolean prop default to true. We don't do that anywhere else as far as I'm aware. Would it make sense to use "negative" here instead, e.g. "dontRefresh", so that undefined and false have the same meaning?
There was a problem hiding this comment.
We have it in
frontend/src/component/feature/StrategyTypes/FlexibleStrategy/FlexibleStrategy.tsxfrontend/src/component/project/ProjectList/ProjectGroup.tsxfrontend/src/component/common/FeatureStatusChip/FeatureStatusChip.tsxfrontend/src/component/changeRequest/ChangeRequest/Changes/Change/StrategyChange.tsx
About the changes
Remove dependency 🫡
Replaced with
5058 LoC + 140 LoTC, usingdate-fnsRelated issues / Prior work
Important files