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鈥檒l occasionally send you account related emails.
Already on GitHub? Sign in to your account
馃悰 no longer clobber koa context #879
Conversation
0c84938
to
004d8ce
Compare
|
||
expect(ctx.body).toContain(myCoolApp); | ||
}); | ||
|
||
// we cannot test this with `createMockContext` because it actually clobbers the proxy methods itself :( |
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.
馃槀
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.
Is there an issue for this? Seems... problematic
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.
updated the PR, I was just doing it wrong and am a dumdum
packages/react-server/README.md
Outdated
import App from '../app'; | ||
|
||
const app = createServer({ | ||
render: (ctx: RenderContext) => <App location={ctx.request.url} /> | ||
render: (ctx: Context) => <App location={ctx.request.url} /> |
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.
You shouldn't ever need to specify the type explicitly here I don't think?
|
||
expect(ctx.body).toContain(myCoolApp); | ||
}); | ||
|
||
// we cannot test this with `createMockContext` because it actually clobbers the proxy methods itself :( |
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.
Is there an issue for this? Seems... problematic
2bdbed7
to
c044e4b
Compare
@michelleyschen can you take this one over? |
59de437
to
c9de133
Compare
c9de133
to
38ae3f3
Compare
Fixes #876
Description
This PR fixes the issue we were having where our use of the spread operator on the
ctx
we got passed by Koa broke a bunch of the proxying / getters on that object, leading things to not work unexpectedly and require delving intoctx.request
properties directly.This PR is actually a breaking change, switching the API for
render
functions from(ctx: Koa.Context & {locale: string}) => JSX
to(ctx: Koa.Context, data: {locale: string})
.