-
-
Notifications
You must be signed in to change notification settings - Fork 51
feat: replace into-stream to Readable.from
#290
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
Changes from 11 commits
5216585
58e1260
2e5810a
9b4a0e4
db10fb4
a610972
fbacb29
9d2685b
6091169
6d63b9b
81bb72c
ce46b67
ea1951b
1664f00
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -8,12 +8,12 @@ | |
| "dependencies": { | ||
| "@fastify/accept-negotiator": "^1.1.0", | ||
| "fastify-plugin": "^4.5.0", | ||
| "into-stream": "^6.0.0", | ||
| "mime-db": "^1.52.0", | ||
| "minipass": "^7.0.2", | ||
| "peek-stream": "^1.1.3", | ||
| "pump": "^3.0.0", | ||
| "pumpify": "^2.0.1" | ||
| "pumpify": "^2.0.1", | ||
| "readable-stream": "^4.5.2" | ||
|
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Not blocking but I just don't see the reason for adding readable-stream if the test suit is passing without it, I remember PRs that removed that package being merged as well I get the stability argument, however with the same point of view one could say that any part of node's API can change in a breaking manner between major versions, no? Then why not install every single built-in module as a package? But maybe readable-stream is special, so just asking to learn
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. We can ensure that bugs in streams not solved in older node versions, like in node14, are not affecting our implementation. |
||
| }, | ||
| "devDependencies": { | ||
| "@fastify/pre-commit": "^2.0.2", | ||
|
|
@@ -26,7 +26,8 @@ | |
| "standard": "^17.1.0", | ||
| "tap": "^16.3.7", | ||
| "tsd": "^0.30.0", | ||
| "typescript": "^5.1.6" | ||
| "typescript": "^5.1.6", | ||
| "undici": "^5.28.3" | ||
| }, | ||
| "scripts": { | ||
| "coverage": "npm run test:unit -- --coverage-report=html", | ||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,48 @@ | ||
| 'use strict' | ||
|
|
||
| const { test } = require('tap') | ||
| const Fastify = require('fastify') | ||
| const fastifyCompress = require('..') | ||
| const { request, setGlobalDispatcher, Agent } = require('undici') | ||
|
|
||
| setGlobalDispatcher(new Agent({ | ||
| keepAliveTimeout: 10, | ||
| keepAliveMaxTimeout: 10 | ||
| })) | ||
|
|
||
| test('should not corrupt the file content', async (t) => { | ||
| // provide 2 byte unicode content | ||
| const twoByteUnicodeContent = new Array(5_000) | ||
| .fill('0') | ||
| .map(() => { | ||
| const random = new Array(10).fill('A').join('🍃') | ||
| return random + '- FASTIFY COMPRESS,🍃 FASTIFY COMPRESS' | ||
| }) | ||
| .join('\n') | ||
| const fastify = new Fastify() | ||
| t.teardown(() => fastify.close()) | ||
|
|
||
| fastify.register(async (instance, opts) => { | ||
| await fastify.register(fastifyCompress) | ||
| // compression | ||
| instance.get('/issue', async (req, reply) => { | ||
| return twoByteUnicodeContent | ||
| }) | ||
| }) | ||
|
|
||
| // no compression | ||
| fastify.get('/good', async (req, reply) => { | ||
| return twoByteUnicodeContent | ||
| }) | ||
|
|
||
| await fastify.listen({ port: 0 }) | ||
|
|
||
| const { port } = fastify.server.address() | ||
| const url = `http://localhost:${port}` | ||
|
|
||
| const response = await request(`${url}/issue`) | ||
| const response2 = await request(`${url}/good`) | ||
| const body = await response.body.text() | ||
| const body2 = await response2.body.text() | ||
| t.equal(body, body2) | ||
| }) |
Uh oh!
There was an error while loading. Please reload this page.