Skip to content

feat(upload/transformFiles): remove unnecessary validation - #1058

Open
an-jello wants to merge 2 commits into
solidjs-community:mainfrom
an-jello:upload/transform-file-remove-uneeded-checks
Open

feat(upload/transformFiles): remove unnecessary validation#1058
an-jello wants to merge 2 commits into
solidjs-community:mainfrom
an-jello:upload/transform-file-remove-uneeded-checks

Conversation

@an-jello

@an-jello an-jello commented Sep 10, 2026

Copy link
Copy Markdown

The current transformFiles function does an unnecessary check for a valid file index number / file existing in an index. This PR removes it.

I've consulted the W3C reference:

  • Indices of FileList "are the numbers in the range zero to one less than the number of File objects"

Summary by CodeRabbit

  • Bug Fixes
    • Improved drag-and-drop state tracking so the dragging indicator updates correctly when entering, leaving, or dropping files.
    • Improved file processing for dropped or selected files, including safer handling of empty input.
  • API Changes
    • Removed the onDragStart, onDragEnd, and onDrag dropzone callbacks.
    • Continued support for onDragEnter, onDragLeave, and onDragOver callbacks.

@changeset-bot

changeset-bot Bot commented Sep 10, 2026

Copy link
Copy Markdown

🦋 Changeset detected

Latest commit: 04c31ee

The changes in this PR will be included in the next version bump.

This PR includes changesets to release 1 package
Name Type
@solid-primitives/upload Patch

Not sure what this means? Click here to learn what changesets are.

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

@coderabbitai

coderabbitai Bot commented Sep 10, 2026

Copy link
Copy Markdown

Review Change StackReview Change Stack

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Advanced

Run ID: 1d819740-49ce-42b4-a3d3-bbce17acdeaa

📥 Commits

Reviewing files that changed from the base of the PR and between c7b608c and c1e461a.

📒 Files selected for processing (5)
  • .changeset/afraid-mails-cry.md
  • .changeset/fine-kids-grin.md
  • packages/upload/src/createDropzone.ts
  • packages/upload/src/helpers.ts
  • packages/upload/src/types.ts
💤 Files with no reviewable changes (1)
  • packages/upload/src/types.ts

Included review availability: Your plan provides up to 4 included reviews per hour; 3 remain after this review.


📝 Walkthrough

Walkthrough

The upload package updates createDropzone drag-state handling, removes obsolete drag callbacks, simplifies transformFiles, and adds release changesets.

Changes

Upload behavior

Layer / File(s) Summary
Dropzone drag state and callback contract
packages/upload/src/types.ts, packages/upload/src/createDropzone.ts, .changeset/afraid-mails-cry.md
createDropzone now updates isDragging on drag enter, drag leave, and drop. It removes dragstart, dragend, and drag listeners and callbacks.
FileList transformation
packages/upload/src/helpers.ts, .changeset/fine-kids-grin.md
transformFiles returns early for null input and iterates FileList indices without numeric validation or missing-file checks.

Estimated code review effort: 2 (Simple) | ~10 minutes

Merge Risk: 🟠 High · up to 04c31

File uploads may fail when FileList indexing yields an absent entry because the new transformation path can pass it to URL.createObjectURL. This should be resolved before merging.

🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 33.33% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 3 functions across 2 files. (2 skipped: 2… Write docstrings for the functions missing them to satisfy the coverage threshold.
✅ Passed checks (4 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title clearly identifies the primary change: removing unnecessary validation from transformFiles.
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.
Full details: Docstring Coverage

Explanation

Docstring coverage is 33.33% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 3 functions across 2 files. (2 skipped: 2 unsupported.)

✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests

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.

@an-jello
an-jello force-pushed the upload/transform-file-remove-uneeded-checks branch from c1e461a to 04c31ee Compare September 10, 2026 15:40
@an-jello

Copy link
Copy Markdown
Author

Whoops, was attempting to use PR stacking but failed :/

Rebased the changes above to be on top of main again

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.

1 participant