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

Update resolvePageComponent helper to work with both glob and glogEager #11

Merged
merged 6 commits into from
Jun 22, 2022

Conversation

Esirei
Copy link
Contributor

@Esirei Esirei commented Jun 9, 2022

This pull request updates the resolvePageComponent inertia helper to work with the object returned both by the glob and globEager imports.

This pull request updates the `resolvePageComponent` inertia helper to work with the object returned both by the glob and globEager imports.
@jessarcher
Copy link
Member

Hi @Esirei, thanks for the PR!

Could you please explain the use case and benefit of this?

@Esirei
Copy link
Contributor Author

Esirei commented Jun 10, 2022

Hi @Esirei, thanks for the PR!

Could you please explain the use case and benefit of this?

Hello @jessarcher , unlike the glob import which compiles each page/component/js into separate files and requests them on demand (lazy loaded), the globEager import compiles all into a single app.js file. Of course this will have a slower initial website load, but faster page responses, it depends on the developer which tradeoff they want.

This PR updates the helper function to work with either technique, I just need to figure out how to tell typescript that the generic cannot be a function to fix the failed tests.

@jessarcher
Copy link
Member

Thanks, @Esirei. That makes sense 👍

@Esirei
Copy link
Contributor Author

Esirei commented Jun 20, 2022

Hi @jessarcher, I finally had some time to get the issue out of the way, also added tests for the helper.

@jessarcher
Copy link
Member

Thanks, @Esirei!

@jessarcher jessarcher merged commit 07d7b9b into laravel:main Jun 22, 2022
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.

3 participants