-
-
Notifications
You must be signed in to change notification settings - Fork 377
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
refactor: code #1770
refactor: code #1770
Conversation
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## master #1770 +/- ##
==========================================
+ Coverage 97.13% 97.55% +0.41%
==========================================
Files 10 10
Lines 454 450 -4
Branches 135 134 -1
==========================================
- Hits 441 439 -2
+ Misses 12 10 -2
Partials 1 1 ☔ View full report in Codecov by Sentry. |
Hmm, I am getting a crash on Node
|
@kumar303 We support 20.x version, can you create reproducible test repo? We have CI and tests on such version... |
Ah, ok, so this is maybe only happening to our application. I'm not sure what's going on or how to reproduce it in isolation. My team is invoking 📄 codeimport type {ServerResponse} from 'http';
import type WebpackDevMiddleware from 'webpack-dev-middleware';
import type {Middleware} from 'koa';
const webpackDevMiddlewareKoaPatched = (
devMiddlewareInstance: ReturnType<typeof WebpackDevMiddleware>,
): Middleware => {
// create promise reference that resolves when the dev middleware is ready to accept connections,
// so on subsequent visits, resolving this same promise would yield it's initial fufilled value
const devMiddlewareReady = new Promise((resolve) => {
devMiddlewareInstance.waitUntilValid(() => resolve(true));
});
return async (ctx, next) => {
const init = new Promise<void>((resolve) => {
devMiddlewareInstance(
ctx.req,
{
end: (content: unknown) => {
ctx.body = content;
resolve();
},
getHeader: ctx.get.bind(ctx),
setHeader: ctx.set.bind(ctx),
locals: ctx.state,
} as unknown as ServerResponse,
() => resolve(next()),
);
});
await Promise.all([init, devMiddlewareReady]);
};
};
export default webpackDevMiddlewareKoaPatched; |
Calling |
hm we don't have official compaibility with koa, but I am glad to add it, sounds like koa doesn't support streams... that is weird to be honetly, because Node.js supports them, if you provide a small full example how you use it I will look and try to fix it ASAP |
This is how we use the code above. import Koa from 'koa'; // 2.15.0
import webpackDevMiddleware from 'webpack-dev-middleware';
import webpackDevMiddlewareKoaPatched from '<the code shown two comments above this one>';
const app = new Koa();
app.use(
webpackDevMiddlewareKoaPatched(
webpackDevMiddleware(multiCompiler, {
writeToDisk: true,
stats: 'errors-only',
publicPath: config.output!.publicPath,
}),
),
); Is this enough to help? Thanks for taking a look 🙏 |
Investigate your problem |
The problem is here:
Because it is not |
D'oh. I see this now. Thanks for the second pair of eyes! |
Not a big problem - #1792, most tests were passed, tomorrow I will create |
Just to report back -- we switched to the koa wrapper and it's working great 🎉 Thanks so much. |
This PR contains a:
Motivation / Use-Case
refactor code
Breaking Changes
No
Additional Info
No