Skip to content
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

feat: getInputProps #746

Open
wants to merge 60 commits into
base: main
Choose a base branch
from
Open

feat: getInputProps #746

wants to merge 60 commits into from

Conversation

juliusmarminge
Copy link
Collaborator

@juliusmarminge juliusmarminge commented Apr 5, 2024

Closes #729

Note

Experimenting a bit without thinking about breaking changes, just providing the best DX. Will come back and see if I can make it backwards compat later

Changelog

Not final, just to keep track what I've tried and such

  • adds a playground example as a playground to play with a bit of everything wihtout having to revert it on every change in a PR to keep it minimal and contrived like the other examples
  • enables hooks and compoennt state to be controlled by the user using the useControlledState from RadixUI
  • ⚠️ BREAKING removes permittedFileInfo
  • ⚠️ BREAKING changes useDropzone signature to take in a route config for easier integration with uploadthing file routes
  • marks bytesToFileSize as a public util for formatting filesize as strings in custom components

Copy link

changeset-bot bot commented Apr 5, 2024

⚠️ No Changeset found

Latest commit: 6d0abe7

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

Copy link

vercel bot commented Apr 5, 2024

The latest updates on your projects. Learn more about Vercel for Git ↗︎

Name Status Preview Comments Updated (UTC)
docs-uploadthing ❌ Failed (Inspect) Jun 26, 2024 8:27am

@@ -242,6 +242,7 @@ component match your design in the exact way you want.
```

<UploadButton
endpoint="mockRoute"
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

having these actually load instead of fetch failing helped reduce some impossible states we had weird paths for just to display in here

$props.__internal_upload_progress ?? 0,
);
const [files, setFiles] = useState<File[]>([]);
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

i think it makes sense for useUploadThing to control state? Especially if the getInputProps should have an onChange that sets files. Felt weird passing those in like getInputProps({ mode, files, setFiles }). By letting the hook own the state we have access to it in getInputProps for "free"

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Have to think how it's used in forms though

Comment on lines 157 to 161
let filesToUpload = pastedFiles;
setFiles((prev) => {
filesToUpload = [...prev, ...pastedFiles];
return filesToUpload;
});
Copy link
Collaborator Author

@juliusmarminge juliusmarminge Apr 6, 2024

Choose a reason for hiding this comment

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

This bug was fixed for dropzone in an earlier PR (#622)

Copy link
Member

@t3dotgg t3dotgg left a comment

Choose a reason for hiding this comment

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

The getInputProps bit looks great to me, hopefully that's not too hard to get up to date. Sorry for letting this slip 🙃

Copy link
Contributor

github-actions bot commented Jun 24, 2024

📦 Bundle size comparison

Bundle Size (gzip) Visualization
Main 39.42KB See Treemap 📊
PR (ae0385d) 39.42KB See Treemap 📊
Diff No change

) => {
const [files, setFiles] = useControllableState({
prop: opts?.files,
onChange: opts?.onFilesChange,
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

note to make this compat with the changes introduced in #886

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

Successfully merging this pull request may close these issues.

feat: useUploadThingInputProps
3 participants