@clerk/upgrade detect binaries and gitignore during scan by rofinn · Pull Request #8324 · clerk/javascript

rofinn · 2026-04-15T20:36:13Z

Description

Fixes #8323 (sorry didn't notice you prefer having issues created first)

As outlined in the issue and linked gist to reproduce the issue, this PR attempts to address 3 particular issues faced while using the @clerk/upgrade cli.

The scanning filename spinner should give the full relative path to make debugging easier.
Uses isbinaryfile to detect if a candidate file is binary (primary issue from @clerk/upgrade stalls on binary build artifacts #8323 )
(Optional) Adds support for using .gitignore files to reduce the scanning scope. And falls through if git is not available or no .gitignore file is present.

NOTES:

Both (2) and (3) include additional tests for detecting binary files, .gitignore files, along with the fallback cases like when git is not available (or not a repo) or if --skip-ignore is used.
I opted to add isbinaryfile as a dependency to packages/upgrade` since it covers more edge cases than my initial implementation.
As per the tinyglobby docs, I'm just shelling-out for the gitignore check.

Checklist

pnpm test runs as expected.
pnpm build runs as expected.
(If applicable) JSDoc comments have been added or updated for any package exports
(If applicable) Documentation has been updated

Just updated the README and the CLI help output.

Type of change

🐛 Bug fix
🌟 New feature
🔨 Breaking change
📖 Refactoring / dependency upgrade / documentation
other: I'd mostly call this a bug fix, but it does add an additional dependency, and the gitignore behaviour could be a breaking change.

NOTE: Since tinyglobby doesn't support gitignore out-of-the-box we're shelling out as is currently recommend in the docs. - https://superchupu.dev/tinyglobby/migration#gitignore - SuperchupuDev/tinyglobby#92 Alternatives, would be to use the `ignore` package, but this seems to work for now without adding an additional dependency. I've left this as a separate commit in case this behaviour isn't desired, but the binary handling is.

changeset-bot · 2026-04-15T20:36:18Z

⚠️ No Changeset found

Latest commit: f1453d3

Merging this PR will not cause a version bump for any packages. If these changes should not result in a new version, you're good to go. If these changes should result in a version bump, you need to add a changeset.

This PR includes no changesets

When changesets are added to this PR, you'll see the packages that this PR includes changesets for and the associated semver types

Click here to learn what changesets are, and how to add one.

Click here if you're a maintainer who wants to add a changeset to this PR

vercel · 2026-04-15T20:36:22Z

@rofinn is attempting to deploy a commit to the Clerk Production Team on Vercel.

A member of the Team first needs to authorize it.

rofinn · 2026-04-15T20:39:03Z

NOTE: I have not created a changeset yet because I think the version bump may depend on whether the last commit (gitignore) is included.

coderabbitai · 2026-04-15T20:40:22Z

📝 Walkthrough

Walkthrough

The changes implement binary file detection and .gitignore support for the upgrade tool's scanning process. A new CLI flag --skip-gitignore is added (default false) to optionally disable .gitignore filtering. The scanner now detects and skips binary files using the isbinaryfile package, retrieves git-ignored files to exclude from scans via git ls-files, displays progress with relative paths instead of basenames, and introduces a debug rendering function. Tests validate the new functionality, CLI help text, error handling for missing git installations or non-git repositories, and the behavior of the skipGitignore option.

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~20 minutes

🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name	Status	Explanation	Resolution
Docstring Coverage	⚠️ Warning	Docstring coverage is 11.11% which is insufficient. The required threshold is 80.00%.	Write docstrings for the functions missing them to satisfy the coverage threshold.

✅ Passed checks (4 passed)

Check name	Status	Explanation
Description check	✅ Passed	The description clearly outlines all three main features implemented and references the linked issue `#8323`, directly addressing the objectives.
Linked Issues check	✅ Passed	The PR successfully addresses all four objectives from issue `#8323`: relative path display in spinner [✓], binary file detection [✓], .gitignore support [✓], and fallback handling [✓].
Out of Scope Changes check	✅ Passed	All code changes directly support the linked issue `#8323` objectives; no out-of-scope changes detected.
Title check	✅ Passed	The title accurately captures the main changes: adding binary file detection and gitignore support during scanning in the upgrade CLI.

_{✏️ Tip: You can configure your own custom pre-merge checks in the settings.}

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

_{Comment @coderabbitai help to get the list of available commands and usage tips.}

rofinn

Just noticed a few test cleanup items on a second review.

rofinn · 2026-04-15T23:05:44Z

+
+  it('skips binary files that are not covered by ignore globs', async () => {
+    const config = await loadConfig('nextjs', 6);
+    config.changes = config.changes.filter(change => change.matcher);


I got a bit lazy while testing. If folks want I can add a concrete config for each test to be more specific?

rofinn · 2026-04-15T23:09:48Z

+    config.changes = config.changes.filter(change => change.matcher);
+
+    const binaryPath = path.join(fixture.path, 'binary');
+    fs.writeFileSync(binaryPath, Buffer.from([0x7f, 0x45, 0x4c, 0x46, 0x00]));


I guess this would be a stronger test if I added Buffer.from('afterSignInUrl') to it.

rofinn added 3 commits April 14, 2026 13:56

Display full relative path during upgrade scanning.

fe0c0e5

Use isbinaryfile to avoid scanning non-text files.

5515502

rofinn changed the title ~~Rf/upgrade exclude node modules~~ @clerk/upgrade detect binaries and gitignore during scan Apr 15, 2026

rofinn commented Apr 15, 2026

View reviewed changes

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

@clerk/upgrade detect binaries and gitignore during scan#8324

@clerk/upgrade detect binaries and gitignore during scan#8324
rofinn wants to merge 3 commits intoclerk:mainfrom
rofinn:rf/upgrade-exclude-node-modules

rofinn commented Apr 15, 2026

Uh oh!

changeset-bot bot commented Apr 15, 2026

Uh oh!

vercel bot commented Apr 15, 2026

Uh oh!

rofinn commented Apr 15, 2026

Uh oh!

coderabbitai bot commented Apr 15, 2026 •

edited

Loading

Walkthrough

Estimated code review effort

❌ Failed checks (1 warning)

Uh oh!

rofinn left a comment

Uh oh!

rofinn Apr 15, 2026

Uh oh!

rofinn Apr 15, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

1 participant

Conversation

rofinn commented Apr 15, 2026

Description

Checklist

Type of change

Uh oh!

changeset-bot bot commented Apr 15, 2026

⚠️ No Changeset found

Uh oh!

vercel bot commented Apr 15, 2026

Uh oh!

rofinn commented Apr 15, 2026

Uh oh!

coderabbitai bot commented Apr 15, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Walkthrough

Estimated code review effort

❌ Failed checks (1 warning)

Uh oh!

rofinn left a comment

Choose a reason for hiding this comment

Uh oh!

rofinn Apr 15, 2026

Choose a reason for hiding this comment

Uh oh!

rofinn Apr 15, 2026

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

1 participant

coderabbitai bot commented Apr 15, 2026 •

edited

Loading