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

create hook useDevice() #595

Open
wants to merge 8 commits into
base: main
Choose a base branch
from
Open

Conversation

guitavano
Copy link
Contributor

No description provided.

props: {
req,
get device() {
return device ??= deviceOf(ctx.request);
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

its not reusing it from ctx, why?

Copy link
Contributor

@mcandeia mcandeia left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I understand the rational behind this change and I know that is sucks to create an inline loader just to access device 90% of the times that you need a loader. However I think this is beneficial for many reasons, I will try to list some of them here but this is not exhaustive, just few of them that I'm thinking right now:

  1. It is less react-compatible component

Meaning that reusability becomes difficult, you can't export this section as an Island, for instance, because it now receives request and for the most of cases request is not available to be used, same for device.

  1. Component props now can receive request and device, which would need us to create extra features to hide/ignore them when generating schema.

  2. users can do it by themself in userland using an app middleware, we can improve the ux for doing it to be a brainless operation

  3. A better alternative more react-compatible can be using hooks? I know there are scenarios where hooks won't work like inside islands that runs client-side, but well our users is facing these problems today

@guitavano
Copy link
Contributor Author

guitavano commented Mar 5, 2024

I followed Candeia instructions and created a Hook.

@guitavano guitavano changed the title add req and device to section props to avoid inline loaders create hook useDevice() Mar 5, 2024
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.

2 participants