refactor: remove concat-stream dependency and modernize MemoryStorage#1356
Open
Phillip9587 wants to merge 1 commit intoexpressjs:mainfrom
Open
refactor: remove concat-stream dependency and modernize MemoryStorage#1356Phillip9587 wants to merge 1 commit intoexpressjs:mainfrom
Phillip9587 wants to merge 1 commit intoexpressjs:mainfrom
Conversation
Replace external concat-stream with a simplified custom implementation without additional deps and convert to ES6 classes
Phillip9587
force-pushed
the
memory-stream
branch
from
5afb6ea to
5d7510a
Compare
There was a problem hiding this comment.
Pull Request Overview
This PR removes the concat-stream dependency from production by implementing a custom ConcatStream class and modernizes the memory storage module with ES6 class syntax.
- Eliminates transitive dependencies (
readable-stream,buffer-from,typedarray) from the production bundle - Implements a custom
ConcatStreamclass with enhanced buffer handling within the memory storage module - Modernizes code structure using ES6 classes instead of prototype-based patterns
Reviewed Changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated 1 comment.
| File | Description |
|---|---|
| storage/memory.js | Replaces concat-stream with custom implementation and converts to ES6 classes |
| package.json | Moves concat-stream from dependencies to devDependencies |
Tip: Customize your code reviews with copilot-instructions.md. Create the file or learn how to get started.
| constructor (cb) { | ||
| super() | ||
| this.body = [] | ||
| this.on('finish', function () { |
There was a problem hiding this comment.
The callback is invoked with this.getBody() but this context may not refer to the ConcatStream instance inside the 'finish' event handler. Use an arrow function or bind the context to ensure this refers to the ConcatStream instance.
Suggested change
| this.on('finish', function () { | |
| this.on('finish', () => { |
This file contains hidden or 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
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
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.
This PR eliminates the
concat-streamdependency by implementing a customConcatStreamclass within the memory storage module.Changes
Moved
concat-streamfrom dependencies to dev dependencies, implemented an internalConcatStreamclass with enhanced buffer handling and modernized the code to ES6 class syntax.Benefits
Removes unnecessary transitive dependencies from the production bundle:
readable-streamv3 - mirrors Node.js v10+ native stream implementationbuffer-from- polyfill forBuffer.from()(available since Node.js v5.10)typedarray- TypedArray polyfill (native support since Node.js v0.10)The custom implementation maintains full compatibility with the existing API while reducing the overall dependency footprint.