-
Notifications
You must be signed in to change notification settings - Fork 34
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
Support Inertia 2.0 deferred props feature #56
Conversation
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.
Thank you @mrfolksy this looks great! I just have a couple minor changes I'd like before we merge, but the structure looks perfect.
inertia/http.py
Outdated
_deferred_props = {} | ||
for key, prop in props.items(): | ||
if isinstance(prop, DeferredProp): | ||
_deferred_props.setdefault(prop.group, []).append(key) |
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.
this is almost too clever, it took me a few reads 😄 I like it though!
inertia/utils.py
Outdated
def model_to_dict(model): | ||
return base_model_to_dict(model, exclude=('password',)) | ||
return base_model_to_dict(model, exclude=("password",)) |
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.
looks like some formatting got messed up in this file
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.
Thanks for spotting this, I realised whilst making this change that my default formatting tools were messing things up, so switched the off. Missed this file!
I've noticed a mix of indentation levels across the project, either 2 or 4 spaces. The below are examples from http.py
.
# indentation = 2 spaces
def partial_keys():
return request.headers.get('X-Inertia-Partial-Data', '').split(',')
# indentation = 4 spaces
def location(location):
return HttpResponse('', status=HTTPStatus.CONFLICT, headers={
'X-Inertia-Location': location,
})
The project may want to consider adding a formatting tool such as black
or ruff
at some point. If you are open to this I could raise a PR....
inertia/utils.py
Outdated
return LazyProp(prop) | ||
|
||
|
||
def defer(prop, group=None): |
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.
README.md
Outdated
return { | ||
'name': lambda: 'Brandon', # this will be rendered on the first load as usual | ||
'data': defer(lambda: some_long_calculation()), | ||
'data1': defer(lambda: some_long_calculation1(), 'group'), |
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.
super small thing, could we add a named key to the example?
'data1': defer(lambda: some_long_calculation1(), group='group')
Thanks for the feedback @BrandonShar, appreciate it, I’ll make the amendments over the next few days. |
…erred props and added additional tests cases
@BrandonShar, changes have been made, let me know if there are any other changes needed. |
Thanks @mrfolksy ! I just corrected conflicts so if you could take a quick look and make sure my commit looks right we'll be good to merge! Also, some type of formatting tool would be awesome here. If you have one you've had success with, I'd be happy to merge it! |
I went ahead and merged it because I'm working on some things right now and I didn't want to conflict again 😄 |
NOTE: I couldn't find much documentation on the mechanics for this feature, so I worked from the
inertia-laravel
package as well as theinertia
client package to figure out what was going on. I'm not very familiar with php or Laravel, but testing with unit tests and a small local app indicates this works.Details
As of version 2.0, Inertia supports the ability to defer the fetching of props until after the page has been initially rendered. Essentially this is similar to the concept of
Lazy props
however Inertia provides convenient frontend components to automatically fetch the deferred props after the page has initially loaded, instead of requiring the user to initiate a reload. For more info see Deferred props in the Inertia documentation.This PR introduces a
defer
function to mark props to be deferred, examples belowGrouping requests
By default, all deferred props get fetched in one request after the initial page is rendered, but you can choose to fetch data in parallel by grouping props together.
In the example above, the
data1
, anddata2
props will be fetched in one request, while thedata
prop will be fetched in a separate request in parallel. Group names are arbitrary strings and can be anything you choose.