-
Notifications
You must be signed in to change notification settings - Fork 0
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: check grid intensity on middleware #26
base: main
Are you sure you want to change the base?
Conversation
so other elements on the page are visible
@arkgor is attempting to deploy a commit to the Sustainable UI Team on Vercel. To accomplish this, @arkgor needs to request access to the Team. Afterwards, an owner of the Team is required to accept their membership request. If you're already a member of the respective Vercel Team, make sure that your Personal Vercel Account is connected to your GitHub account. |
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.
@arkgor like the idea as an exclusive feature for NextJS SUI package. I went with browser API as first approach as it is applicable independent of hosting platform / framework thats being used.
I think we could provide config option to disable browser native geolocation method and give developer freedom to provide location data from source of ther choice - wdyt? We can even expose calculateCarbonIntensity
function to let developer choose where it should happen - in serverless function, edge function or on the client.
Saying that, I think we can make our approach more composable, providing developer possibility to choose where these two actions happen:
IMO default should be browser native and we can provide extensions for specific setup i.e. Vercel + NextJS with middleware that would use the library customization options. Would love to discuss more about this. |
How I think about this package currently is |
@sampittko ahh ok, after our call I noted that we go with next.js first and use everything that next.js offers to build the most wow/effective solution first, I had to misunderstood... 🙈 Then yes, for the just react implementation, it does not make sense. |
@arkgor but it definitely makes sense to provide it for NextJS implementation so if you would like, please go forward with updating the implementation - we can showcase this variant as the more effective one which would be beneficial for the final presentation |
I see value in providing option in library API to source location from different place than browser API. If you do see it too, dont hesitate to go forward! |
Problem
IMO checking grid intensity on client side has couple of potential weak points:
Solution
We can use Next.js middleware (geolocation works well on Vercel for example) to read where the request comes form and perform all the gridIntensity calculations on server-side ... and only if we are not able to do it on server-side, then do it on the browser.