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

feat: support nested and wildcard only props #435

Closed
wants to merge 18 commits into from

Conversation

Tofandel
Copy link

@Tofandel Tofandel commented Aug 7, 2022

This PR adds support for lazy props at any level and retrieving partial data at any depth

Eg: only: ['modal.props.aLazyProp'] or only: ['allNestedLazyProps.*']

There is support for 2 type of wildcards

modal.* => will load all nested props under "modal" and lazy props that are direct children of modal
This wildcard can be continued with the dot notation. Eg:
modal.*.props => will only load all props (lazy or not) named "props" which are 2 level deep within modal

modal.** => will load all lazy and non lazy props under modal, no matter how deep

It's a necessary feature if we want to implement modals and such and very useful for improving performances of very complex pages with a lot of data

Goes by par with inertiajs/inertia#1247

@Tofandel Tofandel changed the title feat: support nested only props feat: support nested and wildcard only props Aug 8, 2022
@innocenzi
Copy link
Contributor

It's a necessary feature if we want to implement modals and such

Why is that?

Can the implementation be simplified using Laravel's Arr::dot and Arr::undot?

@Tofandel
Copy link
Author

Tofandel commented Aug 25, 2022

Because if you implement a modal you usually need a lazy prop modal under which you provide the modal data,
But this modal might have itself lazy data that you want to explicitely request for, so you need to do this

[
  "modal" => Inertia::lazy(fn () => [
    "someData" => "Foo",
    "lazyProp" => Inertia::lazy(fn () => "abc"),
  ])
]

And request the modal with, only: ['modal'] and the lazy prop when inside the modal with ['modal.lazyProp'] and it will just retrieve that lazy prop without changing any other prop

Which is impossible to do with the current implementation, the nested lazy prop will always be evaluated if you request modal


It couldn't be simplified with `Arr::dot` and `Arr::undot` for one simple reason, you'd have to evaluate lazy props before if you want to flatten it in a nested manner and run that on it
Arr::dot([
  "modal" => Inertia::lazy(fn () => [
    "someData" => "Foo",
    "lazyProp" => Inertia::lazy(fn () => "abc"),
  ])
]);

// Will just return
[
  "modal" => Inertia::lazy(fn () => [
    "someData" => "Foo",
    "lazyProp" => Inertia::lazy(fn () => "abc"),
  ])
]
// So back to square 1

it would also perform 100 times worse as you need to perform the flattening and unflattening on the props recursively which could very well have a 10 000 elements especially if you share models with relations instead of the "only" value which is usually only 1-3 elements, you also run the risk of loosing elements (I don't know if Arr::dot preserves empty arrays) but it's overall not a good idea, you really want to work with a tree and evaluate as you traverse for this kind of value visiting

If you mean Arr::undot for the resolveOnly part then it could, but you still need to mark leaf nodes somehow to know at what level to evaluate the lazyProps which means retraversing the tree in the exact same manner, so I don't see a benefit, but yes it could be done and the isLeaf would have to be another parameter passed to the function that can be calculated on the fly if there is no children (but that would break backward compatibility with packed arrays, though they wouldn't have a use now, but it's better to preserve BC)

I can give it a try and see if it doesn't break the tests I made

@Tofandel
Copy link
Author

Tofandel commented Aug 25, 2022

Gave it a try and yes, the issue is that Arr::undot will not handle correctly only: ['modal.lazyProp', 'modal'] (If you want to reload both the lazy and the normal props of modal)

Because it either results in ["modal" => []] or ['modal' => ['lazyProp' => []]] and there is no way to represent this structure without a tree

@mewejo
Copy link

mewejo commented May 30, 2023

Native nested lazy props would be really handy for us, allowing nested dynamic prop registration from other 1st party PHP packages, without polluting the root page props object.

@driesvints
Copy link
Collaborator

Heya. We removed our master branch. Feel free to re-attempt this to 1.x!

@onlime
Copy link

onlime commented May 15, 2024

Heya. We removed our master branch. Feel free to re-attempt this to 1.x!

@Tofandel yes please! This is definitely needed. Greatly appreciating your work towards a glory revival of Inertia!

Waiting for this for quite some time and going to use it in combination with lepikhinb/momentum-modal#18 👏

@Tofandel
Copy link
Author

Tofandel commented May 15, 2024

@onlime Then please feel free to take over my PR on 1.x, keeping in mind that half of the feature has been shipped already #620

I have completely switched from inertia to hybridly (like inertia but much better) due to my complete dissatisfaction with maintenance, lack of progress and disregard for contributors on inertia (I opened about 6 PRs which took several days to make, none of which were ever as much as reviewed and because of the multiple force push it has become impossible to work with and of course I'm only a needle in the haystack of contributors that have the same stories)

I will not spend any more wasted time on inertia after this debacle

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.

None yet

5 participants