-
-
Notifications
You must be signed in to change notification settings - Fork 1.1k
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
chore(js-ts): Convert app/util/streams to TypeScript #11398
Changes from 5 commits
5867311
67e439a
1568ce8
e9ed7e2
ee832ff
e635146
0b97956
8cf7447
043e2ca
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
This file was deleted.
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,88 @@ | ||
import { jsonParseStream, jsonStringifyStream, setupMultiplex } from './streams'; | ||
import Through from 'through2'; | ||
import ObjectMultiplex from '@metamask/object-multiplex'; | ||
import pump from 'pump'; | ||
|
||
jest.mock('through2'); | ||
jest.mock('@metamask/object-multiplex'); | ||
jest.mock('pump'); | ||
|
||
describe('streams', () => { | ||
beforeEach(() => { | ||
jest.clearAllMocks(); | ||
}); | ||
|
||
describe('jsonParseStream', () => { | ||
it('should parse JSON strings', (done) => { | ||
const mockThrough = { | ||
push: jest.fn(), | ||
}; | ||
(Through.obj as jest.Mock).mockImplementation((callback) => { | ||
callback.call(mockThrough, '{"key":"value"}', null, () => {}); | ||
return mockThrough; | ||
}); | ||
|
||
const stream = jsonParseStream(); | ||
expect(Through.obj).toHaveBeenCalled(); | ||
expect(mockThrough.push).toHaveBeenCalledWith({ key: 'value' }); | ||
done(); | ||
}); | ||
}); | ||
|
||
describe('jsonStringifyStream', () => { | ||
it('should stringify objects', (done) => { | ||
const mockThrough = { | ||
push: jest.fn(), | ||
}; | ||
(Through.obj as jest.Mock).mockImplementation((callback) => { | ||
callback.call(mockThrough, { key: 'value' }, null, () => {}); | ||
return mockThrough; | ||
}); | ||
|
||
const stream = jsonStringifyStream(); | ||
expect(Through.obj).toHaveBeenCalled(); | ||
expect(mockThrough.push).toHaveBeenCalledWith('{"key":"value"}'); | ||
done(); | ||
}); | ||
}); | ||
|
||
describe('setupMultiplex', () => { | ||
it('should set up stream multiplexing', () => { | ||
const mockConnectionStream = {} as NodeJS.ReadWriteStream; | ||
const mockMux = {} as NodeJS.ReadWriteStream; | ||
|
||
(ObjectMultiplex as jest.Mock).mockReturnValue(mockMux); | ||
(pump as jest.Mock).mockImplementation((stream1, stream2, stream3, callback) => { | ||
Check failure on line 55 in app/util/streams.test.ts GitHub Actions / scripts (lint)
Check failure on line 55 in app/util/streams.test.ts GitHub Actions / scripts (lint)
|
||
callback(null); | ||
}); | ||
|
||
const result = setupMultiplex(mockConnectionStream); | ||
|
||
expect(ObjectMultiplex).toHaveBeenCalled(); | ||
expect(pump).toHaveBeenCalledWith( | ||
mockConnectionStream, | ||
mockMux, | ||
mockConnectionStream, | ||
expect.any(Function) | ||
); | ||
expect(result).toBe(mockMux); | ||
}); | ||
|
||
it('should handle errors in pump', () => { | ||
const mockConnectionStream = {} as NodeJS.ReadWriteStream; | ||
const mockMux = {} as NodeJS.ReadWriteStream; | ||
const mockError = new Error('Pump error'); | ||
|
||
(ObjectMultiplex as jest.Mock).mockReturnValue(mockMux); | ||
(pump as jest.Mock).mockImplementation((stream1, stream2, stream3, callback) => { | ||
Check failure on line 77 in app/util/streams.test.ts GitHub Actions / scripts (lint)
Check failure on line 77 in app/util/streams.test.ts GitHub Actions / scripts (lint)
|
||
callback(mockError); | ||
}); | ||
|
||
console.warn = jest.fn(); | ||
|
||
setupMultiplex(mockConnectionStream); | ||
|
||
expect(console.warn).toHaveBeenCalledWith(mockError); | ||
}); | ||
}); | ||
}); |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,59 @@ | ||
import Through from 'through2'; | ||
import ObjectMultiplex from '@metamask/object-multiplex'; | ||
import pump from 'pump'; | ||
|
||
type TransformCallback = (error?: Error | null, data?: unknown) => void; | ||
|
||
interface TransformStream extends NodeJS.ReadWriteStream { | ||
push(chunk: unknown, encoding?: BufferEncoding): boolean; | ||
} | ||
|
||
/** | ||
* Returns a stream transform that parses JSON strings passing through | ||
* @return {NodeJS.ReadWriteStream} | ||
*/ | ||
function jsonParseStream(): NodeJS.ReadWriteStream { | ||
return Through.obj(function ( | ||
this: TransformStream, | ||
serialized: string, | ||
_: string, | ||
cb: TransformCallback | ||
) { | ||
this.push(JSON.parse(serialized)); | ||
cb(); | ||
}); | ||
} | ||
|
||
/** | ||
* Returns a stream transform that calls {@code JSON.stringify} | ||
* on objects passing through | ||
* @return {NodeJS.ReadWriteStream} the stream transform | ||
*/ | ||
function jsonStringifyStream(): NodeJS.ReadWriteStream { | ||
return Through.obj(function ( | ||
this: TransformStream, | ||
obj: unknown, | ||
_: string, | ||
cb: TransformCallback | ||
) { | ||
this.push(JSON.stringify(obj)); | ||
cb(); | ||
}); | ||
} | ||
|
||
/** | ||
* Sets up stream multiplexing for the given stream | ||
* @param {NodeJS.ReadWriteStream} connectionStream - the stream to mux | ||
* @return {NodeJS.ReadWriteStream} the multiplexed stream | ||
*/ | ||
function setupMultiplex(connectionStream: NodeJS.ReadWriteStream): NodeJS.ReadWriteStream { | ||
const mux = new ObjectMultiplex(); | ||
pump(connectionStream, mux, connectionStream, (err: Error | null) => { | ||
if (err) { | ||
console.warn(err); | ||
} | ||
}); | ||
return mux as unknown as NodeJS.ReadWriteStream; | ||
} | ||
|
||
export { jsonParseStream, jsonStringifyStream, setupMultiplex }; |
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.
C: It's a shame that it did not make it as a rename so that we could see both JS and TS side by side instead of delete and new.
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.
Unfortunately, we're at the will of how Github diff analyzes the changes
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.
It does it fine in https://github.com/MetaMask/metamask-mobile/pull/11335/files so not sure why here it doesn't see the renaming