-
-
Notifications
You must be signed in to change notification settings - Fork 7.9k
fix(testing): add fallback option to file type validator #15226
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
base: master
Are you sure you want to change the base?
fix(testing): add fallback option to file type validator #15226
Conversation
Pull Request Test Coverage Report for Build 789e8c93-ab4a-4bca-86db-21c38176ac58Details
💛 - Coveralls |
…idator-compatibility
…idator-compatibility
All conflicts have been resolved, and checks are passing. Ready for review |
It seems it's approved but conflict remains, can you please resolve the merge? |
package.json
Outdated
@@ -72,6 +72,7 @@ | |||
"load-esm": "1.0.2", | |||
"object-hash": "3.0.0", | |||
"path-to-regexp": "8.2.0", | |||
"redis": "^4.7.1", |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
why there is a redis dependency?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thank you @itrethan for pointing that out. The redis dependency was added unintentionally during local development and should not have been included in this PR. It is unrelated to the scope of this fix.
I'll remove it immediately to keep the PR focused and clean.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I’ve just resolved the merge conflict and pulled the latest changes. After doing so, I encountered some new issues during the integration tests:
- ESLint errors surfaced in a few files (e.g., formatting issues in the snapshot test).
- Additionally, the code benchmark check failed, though that seems unrelated to the changes in this PR.
The failing integration test is related to the snapshot comparison, although I haven’t modified any relevant logic or snapshots directly. Do you have any suggestions on how to proceed with this?
…idator-compatibility
…b.com/Yasir-Rafique/nest into fix/file-type-validator-compatibility
PR Checklist
Please check if your PR fulfills the following requirements:
PR Type
What kind of change does this PR introduce?
What is the current behavior?
Currently, the FileTypeValidator fails when file-type is unable to determine the MIME type — particularly for file formats with undetectable signatures. This can cause valid uploads to fail unexpectedly.
Additionally, the related unit test did not cover scenarios involving fallback logic or validator option validation.
Issue Number: #15055
What is the new behavior?
Does this PR introduce a breaking change?
Other information
This PR resolves compatibility issues related to MIME type detection and improves developer experience by making file validation more fault-tolerant in real-world uploads. It also keeps dependencies aligned with current CommonJS-based usage patterns.