feat(inertia): add Shared Data support - #2124
Conversation
🦋 Changeset detectedLatest commit: 510ab6c The changes in this PR will be included in the next version bump. This PR includes changesets to release 1 package
Not sure what this means? Click here to learn what changesets are. Click here if you're a maintainer who wants to add another changeset to this PR |
|
Thanks for taking this on! I read through it — this is in good shape, and the type approach in particular turned out better than what I had in mind. On the
|
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #2124 +/- ##
==========================================
+ Coverage 92.44% 92.47% +0.02%
==========================================
Files 119 119
Lines 4274 4291 +17
Branches 1121 1124 +3
==========================================
+ Hits 3951 3968 +17
Misses 288 288
Partials 35 35
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Harness. 🚀 New features to boost your workflow:
|
16b0f2e to
a70e422
Compare
|
@ashunar0 I applied the change because I think restricting share to a synchronous callback is a good choice, especially as it encourages the use of lazy function props. However, since c.render() returns a Promise when asynchronous resolution is required, the current type appears to be inaccurate. I changed it to Response | Promise and added tests, so please take a look. Personally, I don't think this warrants a separate PR, but the commits can be reverted if necessary. I also added page.sharedProps and fixed the typo. This allows us to focus on deciding whether to use the curried form. I personally also think the curried approach is better. If there are places where it could be useful in other middleware, it might be interesting to adopt it there as well. |
a70e422 to
744278a
Compare
|
I forgot to mention sub apps mounted with I assume it's generally understood that this middleware is meant to be registered only on the top-level app, but do you think we should document that explicitly? If Hono itself adds |
There was a problem hiding this comment.
Thanks for the quick turnaround — I've gone through everything.
c.render()return type: Agreed with restrictingshareto a synchronous callback and changing the return type toResponse | Promise<Response>instead of alwaysPromise. Verified the sync fast path is preserved whenshareisn't used and no props need async resolution — no regression there. No need to split this into a separate PR; keeping it here is fine.page.sharedProps: Looks right. Confirmed it always reflects the full set of top-level keys fromshare(), even during a partial reload where most of them get filtered out ofprops— that matches how the client needs it for instant visits.- Curried form: I looked at how this landed and I think it's the right call. Since
Egets inferred from an explicit type annotation on thesharecallback's parameter, we get the same ergonomics as a curried form without introducing a second API shape. Let's go with this. - Typo: Confirmed fixed.
.route()sub-apps: I'd leave this out of this PR's docs. The constraint isn't specific toshare— it's a general@hono/inertialimitation — so it reads better as part of theprovidedocumentation once that discussion settles, rather than bolted onto the Shared Data section here.
This looks good to me. Approving.
|
@ashunar0 I merged the implementation of the curried form after updating the README and JSDoc. I also made one additional change: share is now required when using the curried form, since the curried form is intended to always be used with share. If this looks good to you, I believe the PR is ready to merge. If you would prefer share to remain optional, I’ll revert that change promptly. |
Overview
Add a
shareoption toinertia()so that props shared across pages can be configured with a synchronous callback. Asynchronous values can still use the existing lazy function props mechanism.Changes
feat(inertia): add shared props through a
sharecallbackc.render(), including lazy function propsshareand include them inPagePropssharedPropspage metadata containing the top-level keys returned bysharefix(inertia): correct the
c.render()return typec.render()type toResponse | Promise<Response>and add tests for both return pathsTests and documentation
sharedProps, andc.render()return valuessharedPropsdocumentationInferring types from the
sharecallback return valueThe shared props type is inferred from the return value of the
sharecallback:The inferred type is passed to the
inertiamiddleware throughEnvand reflected in the page props ofc.render(). The shared props type does not need to be specified separately. Type tests usePagePropsForto verify page props for each app without relying on the globalAppRegistry.Shared props for apps mounted with
.route()When another app (sub-app) is mounted on a parent app with
.route(), theinertia({ share })configured on the sub-app works at runtime. However, thesharetype defined in the sub-app's middleware is not propagated to the parent app'sPageProps.To include the sub-app's shared data in
PageProps, the data must be passed explicitly as props toc.render().provideis a middleware helper that stores the shared data in theContextand makes it reusable from each handler.In the non-curried form,
Context<SubEnv>is explicitly specified as the provider parameter, as inprovide('shared', (c: Context<SubEnv>) => ...). In the curried form,provide<SubEnv>()provides the type for the provider'sContext.Example implementation of `provide`
Since
provideis a generic middleware helper that does not affect Inertia's functionality, a PoC PR is planned for the main Hono repository.Curried form
By using currying,
Contextdoes not need to be imported;inertia<SessionEnv>()types theContextpassed to thesharecallback.The curried form is being developed on a separate branch. If adopted, it will either be included in this PR or split into a separate PR.
URL: nkfr26@8feaef5
The author should do the following, if applicable
pnpm changesetat the top of this repo and push the changeset