Skip to content

fix: remove duplicate DEFAULT_RECIPE and SPEED_STEPS from types.ts#733

Closed
MehtabSandhu11 wants to merge 1 commit into
magic-peach:mainfrom
MehtabSandhu11:fix/remove-duplicate-default-recipe
Closed

fix: remove duplicate DEFAULT_RECIPE and SPEED_STEPS from types.ts#733
MehtabSandhu11 wants to merge 1 commit into
magic-peach:mainfrom
MehtabSandhu11:fix/remove-duplicate-default-recipe

Conversation

@MehtabSandhu11
Copy link
Copy Markdown
Contributor

@MehtabSandhu11 MehtabSandhu11 commented May 19, 2026

What

Removed DEFAULT_RECIPE and SPEED_STEPS exports from src/lib/types.ts.

Why

Both constants were defined in two places with conflicting values:

types.ts constants.ts
contrast 0 1
saturation 0 1

contrast: 0 and saturation: 0 are broken FFmpeg eq filter values — they produce a flat grey and greyscale image respectively. Any code importing from types.ts would silently get these broken defaults.

How

Deleted both exports from types.ts. All existing consumers (useVideoEditor.ts, AudioSpeedControl.tsx) already import from constants.ts, so no import paths needed updating.

Closes #726

Please add the appropriate labels for GSSoC so that I can get the points

@vercel
Copy link
Copy Markdown

vercel Bot commented May 19, 2026

@MehtabSandhu11 is attempting to deploy a commit to the magic-peach1's projects Team on Vercel.

A member of the Team first needs to authorize it.

@github-actions github-actions Bot added level:beginner Beginner level - 20 pts type:bug Bug fix labels May 19, 2026
@github-actions
Copy link
Copy Markdown
Contributor

👋 Thanks for your PR, @MehtabSandhu11!

Welcome to Reframe — a browser-based video editor built for everyone 🎬

What happens next

  1. 🤖 Automated checks — build & TypeScript typecheck will run automatically
  2. Vercel preview — a preview deployment will be created (requires maintainer authorization for fork PRs)
  3. 👀 Code review — a maintainer will review your changes
  4. 🚀 Merge — once approved, your PR will be merged!

Quick checklist

  • PR title follows Conventional Commits (e.g. feat: add dark mode)
  • Linked the issue this PR closes (e.g. Closes #123)
  • Tested the changes locally (bun run dev)
  • Build passes (bun run build)

Useful links

Happy coding! 🎉

@github-actions
Copy link
Copy Markdown
Contributor

github-actions Bot commented May 19, 2026

✅ PR Format Check Passed — @MehtabSandhu11

Basic format checks passed. A maintainer will review your code changes.

This does not mean the PR is approved — it just means the format is correct.

@MehtabSandhu11 MehtabSandhu11 force-pushed the fix/remove-duplicate-default-recipe branch from df5e3c4 to 4f5c0fa Compare May 21, 2026 14:21
@magic-peach magic-peach added type:refactor Code refactor gssoc'26 GirlScript Summer of Code 2026 labels May 21, 2026
@magic-peach
Copy link
Copy Markdown
Owner

Hey @MehtabSandhu11! Note that PR #838 (recently merged) already consolidated DEFAULT_RECIPE and SPEED_STEPS into src/lib/constants.ts as part of a broader settings persistence feature.

Please check if this PR still has anything to add after rebasing on main. If the duplicate has already been removed by #838, feel free to close this PR.

@magic-peach
Copy link
Copy Markdown
Owner

Hey @MehtabSandhu11! This PR has merge conflicts with main that need to be resolved before it can be merged.

To fix:

git fetch origin
git checkout <your-branch>
git rebase origin/main
# resolve any conflicts
git push --force-with-lease origin <your-branch>

Code review: The change is correct and well-motivated — removing the duplicate DEFAULT_RECIPE and SPEED_STEPS from types.ts eliminates the silent bug where contrast: 0 and saturation: 0 would produce a flat grey / greyscale image for any consumer importing from types.ts. The constants in constants.ts (with contrast: 1, saturation: 1) are the correct defaults.

Once the merge conflicts are resolved and CI (build, lint, typecheck) all pass, this will be ready to merge.

@magic-peach
Copy link
Copy Markdown
Owner

Hey @MehtabSandhu11! Thanks for catching this. The duplicate DEFAULT_RECIPE / SPEED_STEPS issue has been resolved by PR #965 (fix: correct DEFAULT_RECIPE contrast and saturation to FFmpeg eq neutral values) which is already merged. This PR also has merge conflicts with main. Closing as superseded — your observation was valid and appreciated!

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

Labels

gssoc'26 GirlScript Summer of Code 2026 level:beginner Beginner level - 20 pts type:bug Bug fix type:refactor Code refactor

Projects

None yet

Development

Successfully merging this pull request may close these issues.

[BUG] Title: DEFAULT_RECIPE is duplicated with conflicting contrast and saturation defaults across types.ts and constants.ts

2 participants