Skip to content
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

Add back pageContext function #28

Merged
merged 9 commits into from
Dec 9, 2024
Merged

Add back pageContext function #28

merged 9 commits into from
Dec 9, 2024

Conversation

magne4000
Copy link
Member

Fixes #26

Goal

Do not force users to write universal-middlewares to update the pageContext

@brillout
Copy link
Member

brillout commented Dec 6, 2024

Niceee.

Let's add a BREAKING CHANGE:.

I'd suggest we add a > [!WARNING] **BETA** This package will have relatively frequent breaking changes..

@brillout
Copy link
Member

brillout commented Dec 6, 2024

I'd suggest we prefix all branches with {magne4000,brillout,nitedani}. Makes cleaning up stale branches a lot easier.

@magne4000
Copy link
Member Author

Niceee.

Let's add a BREAKING CHANGE:.

Why though? It does not break the current 0.2.x

I'd suggest we add a > [!WARNING] **BETA** This package will have relatively frequent breaking changes..

👍

@brillout
Copy link
Member

brillout commented Dec 9, 2024

Ah, you're right. I guess the trick is to just update the breaking change of the 0.2.0 release and mention Make sure to update to >= 0.2.4 .

@magne4000 magne4000 requested a review from brillout December 9, 2024 10:50
@magne4000 magne4000 merged commit cbb8e51 into main Dec 9, 2024
4 checks passed
@magne4000 magne4000 deleted the page-context-revert branch December 9, 2024 10:52
@brillout
Copy link
Member

brillout commented Dec 9, 2024

LGTM. I did a bit of polishing (feel free to disagree).

I wonder whether the Node.js object req: IncomingMessage is something Hono users will expect. Maybe we should also provide the whole universal-middleware context & runtime 🤔

@magne4000
Copy link
Member Author

Maybe we should also provide the whole universal-middleware context & runtime

It receives runtime[runtime.adapter] as its parameter, so req: IncomingMessage is just an example. It's not clear though, and the typing must currently be given by the user.
I'll update it so that the typing comes from universal-middleware to make things easier and less confusing.

@magne4000
Copy link
Member Author

Maybe we should also provide the whole universal-middleware context & runtime

It receives runtime[runtime.adapter] as its parameter, so req: IncomingMessage is just an example. It's not clear though, and the typing must currently be given by the user. I'll update it so that the typing comes from universal-middleware to make things easier and less confusing.

Fixed by #29 and published as 0.2.5

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[Discussion] universal-middleware/hono
2 participants