Skip to content

fix: preserve zero hyperframe durations#118

Open
C1-BA-B1-F3 wants to merge 1 commit into
nexu-io:mainfrom
C1-BA-B1-F3:fix/issue-110
Open

fix: preserve zero hyperframe durations#118
C1-BA-B1-F3 wants to merge 1 commit into
nexu-io:mainfrom
C1-BA-B1-F3:fix/issue-110

Conversation

@C1-BA-B1-F3

Copy link
Copy Markdown

Summary

  • preserve explicit duration=0 values from Hyperframes data attributes and inline frame markers
  • avoid falling back to the 3000ms default for valid zero durations
  • add unit coverage for both zero-duration sources

Tests

  • corepack pnpm --dir next exec vitest run src/lib/hyperframes.test.ts
  • corepack pnpm --dir next exec tsc --noEmit

@lefarcen
lefarcen requested a review from PerishCode June 25, 2026 01:51
@lefarcen lefarcen added size/S Small change: 20-99 changed lines risk/medium Medium risk change type/bugfix Bug fix labels Jun 25, 2026

@PerishCode PerishCode left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

@C1-BA-B1-F3 I reviewed the Hyperframes parser change and the new unit coverage for zero-duration values from both data-duration attributes and inline frame markers. The updated parser keeps explicit 0 values from falling through to the default duration, and I checked the Remotion export consumer path for compatibility. Nice focused fix with the right regression coverage.

🔁 Powered by Looper · runner=reviewer · agent=codex · An autonomous AI dev team for your GitHub repos.

@lefarcen

Copy link
Copy Markdown

Hey @C1-BA-B1-F3 — heads-up: PR #119 is also open in this area. Both PRs touch next/src/lib/hyperframes.ts and add regression coverage around zero-duration handling, so it may be worth comparing approaches with @JFJLL while maintainers decide which one lands.

@C1-BA-B1-F3

Copy link
Copy Markdown
Author

Thanks for the heads-up. I will defer to the maintainers on whether this PR or #119 is the cleaner path, and I am happy to adapt or close this one if they prefer the other approach.

@lefarcen

Copy link
Copy Markdown

Thanks for the flexibility here, @C1-BA-B1-F3 — that makes the maintainer decision a lot easier.

No action needed from you right now; we'll let the maintainers choose between #118 and #119, and if they want this one adjusted or closed they'll say so directly here.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

risk/medium Medium risk change size/S Small change: 20-99 changed lines type/bugfix Bug fix

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants