react-dropzone / file-selector

Convert a DragEvent or file input to a list of File objects
MIT License
95 stars 33 forks source link

refactor: clean up file system entry retrieval #112

Open jonkoops opened 1 week ago

jonkoops commented 1 week ago

What kind of change does this PR introduce?

Did you add tests for your changes?

If relevant, did you update the documentation?

Summary Cleans up the code that retrieves and processes file system entries so that the flow is easier to follow. This is done by wrapping methods such as readEntries() and file() and 'promisifying' their callback style to promises, which allows them to be awaited.

Additionally, this PR re-structures the fromDirEntry() function to use the above and uses a while loop to drain the directory reader until there are no more entries to read. This simplifies the code and improves readability as it is no longer required to jump through various functions and manually manage the promise with resolve() and reject() calls.

Type safety is also improved by adding proper type definitions to all the affected code.

Does this PR introduce a breaking change? No

Other information This is a cleanup, but also I am going through and selectively re-writing code to get more familiar with the code-base.

coveralls commented 1 week ago

Pull Request Test Coverage Report for Build 11821216180

Details


Totals Coverage Status
Change from base Build 11817735531: 0.0%
Covered Lines: 89
Relevant Lines: 89

💛 - Coveralls
jonkoops commented 1 week ago

Note this is just the beginning of the code cleanup, I'll be going through the code and refactoring it before releasing the new major so I have a better understanding of how the code works in case bugs arise.

jonkoops commented 1 week ago

@rolandjitsu this one is ready to be reviewed, can I ask you to take a look at it?

jonkoops commented 1 week ago

I see the coverage went down because the tests are not hitting a specific no-op case that did not exist before, let me add a test for that.

jonkoops commented 1 week ago

Ok, test added. Coverage is back at a 100%.

jonkoops commented 6 days ago

@rolandjitsu I agree that isFileSystemDirectoryEntry() and isFileSystemFileEntry() were too overzealous in naming so I've simplified them to isDirectoryEntry() and isFileEntry(), that should be plenty of information since they are always called in a context.