export different file types #26
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Reviewer's Guide by SourceryThis PR improves type definitions and file handling by introducing dedicated type interfaces, moving types to a centralized location, and making minor fixes to file operations. The changes include better TypeScript typing for file-related operations and improved documentation. Sequence diagram for storing files locallysequenceDiagram
participant EventHandler
participant StorageUtils
EventHandler->>StorageUtils: storeFileLocally(file, 12, '/specificFolder')
StorageUtils->>StorageUtils: mkdir(location + filelocation)
StorageUtils->>StorageUtils: writeFile(location + filelocation + filename, binaryString)
StorageUtils-->>EventHandler: return filename
Updated class diagram for file handling typesclassDiagram
class ServerFile {
+string name
+string content
+string size
+string type
+string lastModified
}
class ClientFile {
+string|ArrayBuffer|null|undefined content
+string name
+number lastModified
}
class ModuleOptions {
+string mount
+string version
}
File-Level Changes
Tips and commandsInteracting with Sourcery
Customizing Your ExperienceAccess your dashboard to:
Getting Help
|
Closed
![sourcery-ai[bot]](https://avatars.githubusercontent.com/in/48477?s=60&v=4){kind=small}
There was a problem hiding this comment.
Hey @Korny666 - I've reviewed your changes - here's some feedback:
Overall Comments:
- The
as anytype assertion in storage.ts should be avoided. Consider properly typing the binaryString variable instead of bypassing type checking. - Removing files.value.splice(0) from handleFileInput could lead to memory leaks or unexpected behavior as files accumulate. Consider keeping the array clear operation or document why it's no longer needed.
Here's what I looked at during the review
- 🟡 General issues: 1 issue found
- 🟢 Security: all looks good
- 🟢 Testing: all looks good
- 🟢 Complexity: all looks good
- 🟢 Documentation: all looks good
Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.
| @@ -24,7 +25,6 @@ export default function () { | |||
| } | |||
|
|
|||
| const handleFileInput = async (event: any) => { | |||
There was a problem hiding this comment.
issue (bug_risk): Removed files.value.splice(0) could lead to file accumulation
Without clearing the files array, multiple file selections will keep adding to the array, potentially causing memory issues.
|
looks great! thanks for your contribution I will be pushing the update to npm soon. I was wondering about something which is a breaking change I wanted to push, do you know the most optimal method of warning the users on a breaking change without displaying a warning in the terminal over and over? |
Closed
|
Hey ali, what do you want the user to look for? do you want to inform the
user better? let me know what you want to achieve :)
Am Do., 31. Okt. 2024 um 16:18 Uhr schrieb Ali Sokkar <
***@***.***>:
… looks great! thanks for your contribution I will be pushing the update to
npm soon. I was wondering about something which is a breaking change I
wanted to push, do you know the most optimal method of warning the users on
a breaking change without displaying a warning in the terminal over and
over?
—
Reply to this email directly, view it on GitHub
<#26 (comment)>,
or unsubscribe
<https://github.com/notifications/unsubscribe-auth/ADAR7CI4QVIHQIQY654VWFLZ6JC5ZAVCNFSM6AAAAABQ6P5UM6VHI2DSMVQWIX3LMV43OSLTON2WKQ3PNVWWK3TUHMZDINJQGE2DQMJRG4>
.
You are receiving this because you were mentioned.Message ID:
***@***.***>
|
|
I just want the user to know that there is a breaking change that you might need to look into but I don't want to add a console.warn() function that keeps warning the user each time they rerun the server |
|
Hey,
short question - i use pnpm nuxt dev i cannot use the file-storage module
for bigger files. Did you encounter the same problem? If yes, were you able
to fix it? if i build and run my app in a target environment it has no
problems.
did you mean npm depricate? this would warn the user when installing an
older version.
and "just" increase the major version for breaking changes. this is the way
i know it is not compatible.
shot summary what i am aware of:
*Semantic Versioning (SemVer) in npm Package Versioning:*
In npm (Node Package Manager), package versions follow the *Semantic
Versioning (SemVer)* convention, which uses a three-part version number in
the format *X.Y.Z*:
1.
*MAJOR version (X):*
- *What it represents:* Increments when you make incompatible API
changes or introduce breaking changes that are not backward-compatible.
- *Impact on users:* Updating to a new MAJOR version may require
users to modify their code to accommodate the changes.
- *Example:* Changing from 1.5.2 to 2.0.0 indicates significant
changes that could break existing functionality if not addressed.
2.
*MINOR version (Y):*
- *What it represents:* Increments when you add functionality in a
backward-compatible manner.
- *Impact on users:* Users can update to the new MINOR version
without changing existing code, but they can take advantage of
new features
if desired.
- *Example:* Changing from 1.5.2 to 1.6.0 signifies new features or
improvements that don't break existing APIs.
3.
*PATCH version (Z):*
- *What it represents:* Increments when you make backward-compatible bug
fixes.
- *Impact on users:* Safe to update without any code changes; users
benefit from bug fixes and minor enhancements.
- *Example:* Changing from 1.5.2 to 1.5.3 indicates that bugs have
been fixed, but no new features or breaking changes have been introduced.
*Summary:*
- *MAJOR (X):* Breaking changes; incompatible API modifications.
- *MINOR (Y):* New features; backward-compatible enhancements.
- *PATCH (Z):* Bug fixes; backward-compatible corrections.
*Why It's Important:*
Semantic Versioning provides a clear and predictable way to convey the
nature of changes in new releases. It helps users understand the potential
impact of updating a package and manage dependencies more effectively.
*Example Scenario:*
- If you release version *2.0.0* of your package:
- *Users should be cautious* and review the release notes or
migration guides because there are breaking changes.
- If you release version *1.6.0*:
- *Users can update confidently*, knowing that new features are
available without breaking existing functionality.
- If you release version *1.5.3*:
- *Users should update to benefit from bug fixes*, with no risk to
their existing code.
*Additional Notes:*
- *Pre-release Versions:* You can denote pre-release versions using
identifiers like -alpha, -beta, or -rc (e.g., 1.0.0-alpha).
- *Build Metadata:* Optional build metadata can be added after a +,
e.g., 1.0.0+001.
*Version Constraints in Dependencies:*
When specifying package dependencies in package.json, you can define
version ranges:
- "^1.5.0": Accepts any version >=1.5.0 <2.0.0.
- "~1.5.0": Accepts any version >=1.5.0 <1.6.0.
Cheers
Korny
Am Di., 5. Nov. 2024 um 20:37 Uhr schrieb Ali Sokkar <
***@***.***>:
… I just want the user to know that there is a breaking change that you
might need to look into but I don't want to add a console.warn() function
that keeps warning the user each time they rerun the server
—
Reply to this email directly, view it on GitHub
<#26 (comment)>,
or unsubscribe
<https://github.com/notifications/unsubscribe-auth/ADAR7CMDEWFRGWXZFGXMQ53Z7EM7VAVCNFSM6AAAAABQ6P5UM6VHI2DSMVQWIX3LMV43OSLTON2WKQ3PNVWWK3TUHMZDINJYGAYDQMJQGE>
.
You are receiving this because you were mentioned.Message ID:
***@***.***>
|
|
Hello, Regarding the pnpm issue I haven't encountered such an issue and I'd like to get more logs on the topic or perhaps create a reproduction of the issue through this link. About the semantic versioning I was planning on releasing a totally complete version of Nuxt File Storage that has file browser features with even more documentation and some other features that I already made documentation for on file-storage.nuxt.space in addition to a logo change so I will not be touching the major version until I have time to develop the other ideas I had in mind. but for now I believe your suggestion of making it backwards compatible while also implementing the other features I had in mind would be a great move. Thanks for your help! |
# for free
to join this conversation on GitHub.
Already have an account?
# to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Sorry but my fork did not want to update...
@NyllRE What do you think of this?
Summary by Sourcery
Add support for exporting different file types by introducing new TypeScript interfaces for ServerFile and ClientFile. Refactor storage utilities and module type exports to utilize these new types, enhancing type safety and consistency across the codebase.
New Features:
Enhancements: