Skip to content

fix(desktop): handle duration_in_seconds correctly in timestamp #3321

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 6 commits into from
Mar 28, 2025

Conversation

kovsu
Copy link
Contributor

@kovsu kovsu commented Mar 28, 2025

Before After
CleanShot 2025-03-28 at 09 50 23 CleanShot 2025-03-28 at 09 50 29 CleanShot 2025-03-28 at 09 50 02

@follow-reviewer-bot
Copy link

Thank you for your contribution. We will review it promptly.

Copy link

vercel bot commented Mar 28, 2025

@kovsu is attempting to deploy a commit to the RSS3 Team on Vercel.

A member of the Team first needs to authorize it.

@follow-reviewer-bot
Copy link

Suggested PR Title:

fix(TimeStamp): format media duration from string to number

Change Summary:
Updated TimeStamp component to format media duration correctly when it is a string representation of a number. This resolves potential type issues and ensures accurate handling of media duration.

Code Review:

Code Review: Change Requests

apps/desktop/src/renderer/src/modules/renderer/components/TimeStamp.tsx

  1. Line 11:

    • Issue: The formatTimeToSeconds function is being used, but there is no validation to ensure the input type matches what the function expects. While there is a comment indicating the type mismatch (@ts-expect-error durationInSeconds is string), it does not seem ideal to rely on a TypeScript error suppression for this logic. If duration_in_seconds is a string but contains invalid content (e.g., non-numeric characters), formatTimeToSeconds might fail unexpectedly or produce incorrect results.
    • Change Request: Consider adding validation or type-checking logic before calling formatTimeToSeconds. For example, ensure the string represents a valid time format or numeric value.
  2. Line 12:

    • Issue: Suppressing TypeScript errors with @ts-expect-error should generally be avoided unless absolutely necessary. Instead, the code should resolve any type mismatch explicitly, such as by ensuring the type of duration_in_seconds is correct upstream where the data is being fetched or processed. Suppressing errors can lead to unexpected runtime issues and reduce maintainability.
    • Change Request: Resolve the type mismatch by fixing the type definitions for duration_in_seconds in the corresponding type interface or schema, ensuring TypeScript correctly infers the data type.

Addressing these issues will improve the reliability and maintainability of the code.

@hyoban hyoban marked this pull request as draft March 28, 2025 03:06
@hyoban hyoban marked this pull request as ready for review March 28, 2025 03:17
@hyoban hyoban enabled auto-merge (squash) March 28, 2025 03:17
@hyoban hyoban merged commit 88c3cb6 into RSSNext:dev Mar 28, 2025
4 of 6 checks passed
@follow-reviewer-bot
Copy link

Thank you for your contribution! 🎉

Your pull request has been merged and we really appreciate your help in making this project better. We hope to see more contributions from you in the future! 💪

@kovsu kovsu deleted the fix/time-stamp-media-duration branch March 28, 2025 03:22
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants