Add support for always and except
#152
Conversation
| def drop_partial_except_keys(hash) | ||
| partial_except_keys.each do |key| | ||
| parts = key.to_s.split('.').map(&:to_sym) | ||
| *initial_keys, last_key = parts |
There was a problem hiding this comment.
this supports dot notation, yea? so like except: ['users.some_association.expensive_thing_to_compute']? [edit: I see the dot notation in the specs! question below remains]
do the other adapters support that? I don't see it documented anywhere.
either way, if we support it for :except, should we also support it for :only? (genuine question)
There was a problem hiding this comment.
do the other adapters support that? I don't see it documented anywhere.
yup, Laravel adapter supports that
if we support it for :except, should we also support it for :only?
Laravel adapter does not 😄 To be honest the whole except + only thing is poorly documented, we probably should help the core team make it happen.
There was a problem hiding this comment.
Actually they reverted back dot notation for only: inertiajs/inertia-laravel#641
While this would be a nice feature to eventually have, this particular implementation introduced some breaking changes.
There was a problem hiding this comment.
Ok, i dug into this a bit. TL;DR: I don't think those issues will affect our implementation.
Best I can tell (caveat: using my brain's PHP interpreter) is that the original implementation of Laravel's share shallow merged all data. However, dot notation allowed you to deep merge dot-notated shared data. Not sure whether that was intentional or a happy accident since I don't think the feature was ever documented. When dot notation was added to only and except, it changed the execution order and changed the behavior such that all shared data was a shallow merge. So, people relying on the deep merge behavior had breaking changes. (There was another reported but with lazy props that I didn't fully diagnose... looks to be something related to array manipulation quirks in PHP).
InertiaRails 1) has explicit patterns for deep merging shared data and 2) doesn't support dot notation in shared data anyway, so we should be totally fine.
I don't see a reason why we couldn't support dot notation for only as well. Do you think it's worth adding? Seems like it'd be nice if some prop had multiple expensive child props and you only wanted to grab one of them.
There was a problem hiding this comment.
Yup, let's try adding support for ’only’ in a separate PR 👍
|
|
||
| module InertiaRails | ||
| # Base class for all props. | ||
| class BaseProp |
There was a problem hiding this comment.
I like this. Half-baked thought: I wonder if we could move more rendering logic into the Prop classes. The computed_props method is doing more and more things in a sort of procedural style. Maybe it could iterate over the props one time and each instance would know what to do with itself.
(Not suggesting changes, more just thinking out loud)
This PR "upstreams" some features from #132 which are related to Inertia.js 1.x, it includes:
InertiaRails.alwaysprop from inertia.js 1.3: https://inertiajs.com/partial-reloads#lazy-data-evaluationexceptoption from inertia.js 1.3: https://inertiajs.com/partial-reloads#except-certain-propsInertiaRails.lazy(false)andInertiaRails.lazy(nil)raising errorsrequest.inertia_partial?update: with introduction of theX-Inertia-Partial-Exceptheader,X-Inertia-Partial-Datais now can be omitted whenrouter.visit(path, {except: 'foo'})is called.PR is intended to be reviewed commit by commit.