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

fix: add path type in acceptedFiles #1178

Open
wants to merge 4 commits into
base: master
Choose a base branch
from

Conversation

aslan-alt
Copy link

@aslan-alt aslan-alt commented Apr 24, 2022

What kind of change does this PR introduce?

  • Bug Fix
  • Feature
  • Refactoring
  • Style
  • Build
  • Chore
  • Documentation
  • CI

Did you add tests for your changes?

  • Yes, my code is well tested
  • Not relevant

If relevant, did you update the documentation?

  • Yes, I've updated the documentation
  • Not relevant

Summary
fix ts error

Does this PR introduce a breaking change?
No

Other information

@rolandjitsu
Copy link
Collaborator

rolandjitsu commented Apr 25, 2022

@aslan-alt we cannot do this because the type of the files depends on what the getFilesFromEvent returns. So we use T extends File in places where we return file objects.

@rolandjitsu rolandjitsu reopened this Apr 25, 2022
typings/tests/hook.tsx Outdated Show resolved Hide resolved
@aslan-alt aslan-alt changed the title Jingsong.xiong/fix files ts error Fix: fix ts error Apr 26, 2022
@aslan-alt aslan-alt changed the title Fix: fix ts error fix: fix ts error Apr 26, 2022
@aslan-alt aslan-alt changed the title fix: fix ts error fix: add path type in acceptedFiles Apr 26, 2022
@coveralls
Copy link

coveralls commented Apr 26, 2022

Pull Request Test Coverage Report for Build 437f9957b0d2dbc07b057757c7425326eeb8cedb-PR-1178

  • 0 of 0 changed or added relevant lines in 0 files are covered.
  • No unchanged relevant lines lost coverage.
  • Overall coverage remained the same at 98.889%

Totals Coverage Status
Change from base Build 2a71cc9f24681c0a79781305b87d413e190e14bc: 0.0%
Covered Lines: 237
Relevant Lines: 237

💛 - Coveralls

typings/tests/hook.tsx Outdated Show resolved Hide resolved
@aslan-alt aslan-alt force-pushed the jingsong.xiong/fix-files-ts-error branch 2 times, most recently from 4f25d6a to d164104 Compare April 28, 2022 01:28
Comment on lines 69 to 70
draggedFiles: (File & { path?: string })[];
acceptedFiles: (File & { path?: string })[];
Copy link
Collaborator

Choose a reason for hiding this comment

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

Isn't FileWithPath the same thing?

Suggested change
draggedFiles: (File & { path?: string })[];
acceptedFiles: (File & { path?: string })[];
draggedFiles: FileWithPath[];
acceptedFiles: FileWithPath[];

Could you try that and see if you still get errors?

Copy link
Author

@aslan-alt aslan-alt Apr 30, 2022

Choose a reason for hiding this comment

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

yeah,I always see this error, see this demo

Copy link
Collaborator

Choose a reason for hiding this comment

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

@aslan-alt could you update to v14 and see if the same issue happens? I'll take a look later this week.

Copy link
Collaborator

Choose a reason for hiding this comment

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

I just tried it out and if you set:

 acceptedFiles: FileWithPath[];

as suggested, the error is gone.

So please update that and also set the FileRejection.file prop type to the same type.

Copy link
Author

Choose a reason for hiding this comment

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

okay

Copy link
Author

Choose a reason for hiding this comment

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

need approve

@aslan-alt aslan-alt force-pushed the jingsong.xiong/fix-files-ts-error branch from 7e5235d to 8bae3a1 Compare May 4, 2022 01:24
@aslan-alt aslan-alt force-pushed the jingsong.xiong/fix-files-ts-error branch from 8bae3a1 to f8f5a42 Compare May 4, 2022 01:37
@GiancarlosIO
Copy link

👀

@aguscha333
Copy link

Any update on when this will be merged?

@Crayon-ShinChan
Copy link

Will this PR be merged? I noticed that this project hasn't accepted any new PRs for the past two years. guys, what's going on?

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.

None yet

7 participants