fix: add longitude, latitude, and timezone to RequestData.geo #1777

nickytonline · 2022-11-15T15:58:05Z

Summary

Test plan

Visit the Deploy Preview (insert link to specific page) ...

Relevant links (GitHub issues, Notion docs, etc.) or a picture of cute animal

Closes https://github.com/netlify/pod-ecosystem-frameworks/issues/305
Relates to https://github.com/netlify/pod-compute/issues/270
Relates to netlify/cli#5231

Standard checks:

Check the Deploy Preview's Demo site for your PR's functionality
Add docs when necessary

🧪 Once merged, make sure to update the version if needed and that it was published correctly.

netlify · 2022-11-15T15:58:10Z

✅ Deploy Preview for netlify-plugin-nextjs-nx-monorepo-demo ready!

Name	Link
🔨 Latest commit	`36c0835`
🔍 Latest deploy log	https://app.netlify.com/sites/netlify-plugin-nextjs-nx-monorepo-demo/deploys/637772aaa0fb4c0009a9c114
😎 Deploy Preview	https://deploy-preview-1777--netlify-plugin-nextjs-nx-monorepo-demo.netlify.app
📱 Preview on mobile	Toggle QR Code... Use your smartphone camera to open QR code link.

To edit notification comments on pull requests, go to your Netlify site settings.

netlify · 2022-11-15T15:58:11Z

✅ Deploy Preview for netlify-plugin-nextjs-static-root-demo ready!

Name	Link
🔨 Latest commit	`36c0835`
🔍 Latest deploy log	https://app.netlify.com/sites/netlify-plugin-nextjs-static-root-demo/deploys/637772aa171bba00092ad5d9
😎 Deploy Preview	https://deploy-preview-1777--netlify-plugin-nextjs-static-root-demo.netlify.app
📱 Preview on mobile	Toggle QR Code... Use your smartphone camera to open QR code link.

To edit notification comments on pull requests, go to your Netlify site settings.

netlify · 2022-11-15T15:59:38Z

❌ Deploy Preview for nextjs-plugin-custom-routes-demo failed.

Name	Link
🔨 Latest commit
🔍 Latest deploy log	https://app.netlify.com/sites/nextjs-plugin-custom-routes-demo/deploys/6377b009105c7100768b4c62

netlify · 2022-11-15T15:59:43Z

✅ Deploy Preview for next-plugin-edge-middleware ready!

Name	Link
🔨 Latest commit	`36c0835`
🔍 Latest deploy log	https://app.netlify.com/sites/next-plugin-edge-middleware/deploys/637772aa16a90a000a88b002
😎 Deploy Preview	https://deploy-preview-1777--next-plugin-edge-middleware.netlify.app
📱 Preview on mobile	Toggle QR Code... Use your smartphone camera to open QR code link.

To edit notification comments on pull requests, go to your Netlify site settings.

netlify · 2022-11-15T16:00:23Z

❌ Deploy Preview for netlify-plugin-nextjs-next-auth-demo failed.

Name	Link
🔨 Latest commit
🔍 Latest deploy log	https://app.netlify.com/sites/netlify-plugin-nextjs-next-auth-demo/deploys/6377b02bdd2ac3005e8f2868

netlify · 2022-11-15T16:00:54Z

❌ Deploy Preview for netlify-plugin-nextjs-export-demo failed.

Name	Link
🔨 Latest commit
🔍 Latest deploy log	https://app.netlify.com/sites/netlify-plugin-nextjs-export-demo/deploys/6377b02ef9f514006c9e8cf4

netlify · 2022-11-15T16:00:55Z

❌ Deploy Preview for next-plugin-canary failed.

Name	Link
🔨 Latest commit
🔍 Latest deploy log	https://app.netlify.com/sites/next-plugin-canary/deploys/6377b0321645e000508bf000

netlify · 2022-11-15T16:00:55Z

✅ Deploy Preview for next-i18next-demo ready!

Name	Link
🔨 Latest commit	`36c0835`
🔍 Latest deploy log	https://app.netlify.com/sites/next-i18next-demo/deploys/637772aa4d3a180008b8ca25
😎 Deploy Preview	https://deploy-preview-1777--next-i18next-demo.netlify.app
📱 Preview on mobile	Toggle QR Code... Use your smartphone camera to open QR code link.

To edit notification comments on pull requests, go to your Netlify site settings.

netlify · 2022-11-15T16:01:07Z

❌ Deploy Preview for nextjs-plugin-next-11-demo failed.

Name	Link
🔨 Latest commit	`36c0835`
🔍 Latest deploy log	https://app.netlify.com/sites/nextjs-plugin-next-11-demo/deploys/637772aacf03520009147fd3

netlify · 2022-11-15T16:01:17Z

✅ Deploy Preview for netlify-plugin-nextjs-demo ready!

Name	Link
🔨 Latest commit	`36c0835`
🔍 Latest deploy log	https://app.netlify.com/sites/netlify-plugin-nextjs-demo/deploys/637772aa98e0fa0008952734
😎 Deploy Preview	https://deploy-preview-1777--netlify-plugin-nextjs-demo.netlify.app
📱 Preview on mobile	Toggle QR Code... Use your smartphone camera to open QR code link.

To edit notification comments on pull requests, go to your Netlify site settings.

nickytonline

In Netlify context, longitude and latitude are of type number, but Next.js types them as string | undefined. See https://nextjs.org/docs/api-reference/next/server#nextrequest. I wonder if on our end we should convert them to string to avoid any issues on the Next.js side. I do find it odd that they're not treated as numbers on the Next side though.

packages/runtime/src/templates/edge/runtime.ts

…de this

…-lat-to-geo

nickytonline · 2022-11-18T00:48:31Z

I believe the failures for the checks are Next 11 related.

packages/next/src/middleware/request.ts

sarahetter · 2022-11-18T14:03:33Z

packages/runtime/src/templates/edge/runtime.ts

@@ -63,10 +64,13 @@ const handler = async (req: Request, context: Context) => {
    return
  }

-  const geo = {
+  const geo: RequestData['geo'] = {


sarahetter

🎉

nickytonline · 2022-11-18T14:16:37Z

There's Next 11 demo sites failing, but just going to see about the other site failures.

nickytonline · 2022-11-18T16:19:41Z

Looks like all things Cypress are passing, so I think all the failures are related to Next 11 tests.

ascorbic · 2022-11-18T17:00:17Z

If you look at details you can see that several demo sites failed to build. I don't think it's the fault of this PR (I think it's npm-related) but Cypress tests alone won't say that everything is passing.

nickytonline · 2022-11-18T18:10:56Z

If you look at details you can see that several demo sites failed to build. I don't think it's the fault of this PR (I think it's npm-related) but Cypress tests alone won't say that everything is passing.

Are the Cypress tests passing because it's not using the DEPLOY URL? I remember @MarcL mentioning something about that on Slack. Also, I checked on #1781 and all the checks are good aside from the now failures for the Next 11 checks.

github-actions bot added the type: bug code to address defects in shipped code label Nov 15, 2022

nickytonline self-assigned this Nov 15, 2022

nickytonline force-pushed the nickytonline/add-long-lat-to-geo branch from 084127f to 6c44ddc Compare November 15, 2022 16:49

nickytonline changed the title ~~fix: add longitude and latitude to RequestData.geo~~ fix: add longitude, latitude, and timezone to RequestData.geo Nov 15, 2022

nickytonline marked this pull request as ready for review November 15, 2022 21:02

nickytonline requested a review from a team November 15, 2022 21:02

nickytonline marked this pull request as draft November 15, 2022 21:17

nickytonline force-pushed the nickytonline/add-long-lat-to-geo branch 2 times, most recently from 745e278 to 9b296f0 Compare November 16, 2022 00:58

nickytonline commented Nov 16, 2022

View reviewed changes

nickytonline force-pushed the nickytonline/add-long-lat-to-geo branch from 9b296f0 to 129a1d1 Compare November 16, 2022 01:13

fix: add longitude, latitude to RequestData.geo

949689a

nickytonline force-pushed the nickytonline/add-long-lat-to-geo branch from 129a1d1 to 949689a Compare November 16, 2022 01:14

nickytonline commented Nov 16, 2022

View reviewed changes

packages/runtime/src/templates/edge/runtime.ts Show resolved Hide resolved

chore: removed setting a header for timezone as Next.js doesn't provi…

ac259b4

…de this

MarcL linked an issue Nov 16, 2022 that may be closed by this pull request

[Bug]: middleware request.geo object is empty #1751

Closed

2 tasks

nickytonline added 3 commits November 17, 2022 13:19

Merge remote-tracking branch 'origin/main' into nickytonline/add-long…

84be541

…-lat-to-geo

Merge branch 'main' into nickytonline/add-long-lat-to-geo

223937f

chore: added timezone to geo data

aa59d03

nickytonline marked this pull request as ready for review November 17, 2022 22:05

nickytonline added 4 commits November 17, 2022 17:55

test: updated geo test for request

42381b7

chore: imported correct NextRequest type

f3af4ad

chore: removed unused import

fcf020f

chore: updated api route for geo test

b658c0c

ascorbic reviewed Nov 18, 2022

View reviewed changes

packages/next/src/middleware/request.ts Outdated Show resolved Hide resolved

chore: cleaned up NextRequest type

4d0801f

nickytonline force-pushed the nickytonline/add-long-lat-to-geo branch from 2926672 to 4d0801f Compare November 18, 2022 11:54

Merge branch 'main' into nickytonline/add-long-lat-to-geo

36c0835

ascorbic reviewed Nov 18, 2022

View reviewed changes

packages/next/src/middleware/request.ts Show resolved Hide resolved

ascorbic approved these changes Nov 18, 2022

View reviewed changes

sarahetter reviewed Nov 18, 2022

View reviewed changes

sarahetter approved these changes Nov 18, 2022

View reviewed changes

nickytonline merged commit 3f35549 into main Nov 18, 2022

nickytonline deleted the nickytonline/add-long-lat-to-geo branch November 18, 2022 16:20

token-generator-app bot mentioned this pull request Nov 18, 2022

chore: release main #1781

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

fix: add longitude, latitude, and timezone to RequestData.geo #1777

fix: add longitude, latitude, and timezone to RequestData.geo #1777

nickytonline commented Nov 15, 2022 •

edited

Loading

netlify bot commented Nov 15, 2022 •

edited

Loading

netlify bot commented Nov 15, 2022 •

edited

Loading

netlify bot commented Nov 15, 2022 •

edited

Loading

netlify bot commented Nov 15, 2022 •

edited

Loading

netlify bot commented Nov 15, 2022 •

edited

Loading

netlify bot commented Nov 15, 2022 •

edited

Loading

netlify bot commented Nov 15, 2022 •

edited

Loading

netlify bot commented Nov 15, 2022 •

edited

Loading

netlify bot commented Nov 15, 2022 •

edited

Loading

netlify bot commented Nov 15, 2022 •

edited

Loading

nickytonline left a comment

nickytonline commented Nov 18, 2022

sarahetter Nov 18, 2022

sarahetter left a comment

nickytonline commented Nov 18, 2022

nickytonline commented Nov 18, 2022

ascorbic commented Nov 18, 2022

nickytonline commented Nov 18, 2022 •

edited

Loading

fix: add longitude, latitude, and timezone to RequestData.geo #1777

fix: add longitude, latitude, and timezone to RequestData.geo #1777

Conversation

nickytonline commented Nov 15, 2022 • edited Loading

Summary

Test plan

Relevant links (GitHub issues, Notion docs, etc.) or a picture of cute animal

Standard checks:

netlify bot commented Nov 15, 2022 • edited Loading

✅ Deploy Preview for netlify-plugin-nextjs-nx-monorepo-demo ready!

netlify bot commented Nov 15, 2022 • edited Loading

✅ Deploy Preview for netlify-plugin-nextjs-static-root-demo ready!

netlify bot commented Nov 15, 2022 • edited Loading

❌ Deploy Preview for nextjs-plugin-custom-routes-demo failed.

netlify bot commented Nov 15, 2022 • edited Loading

✅ Deploy Preview for next-plugin-edge-middleware ready!

netlify bot commented Nov 15, 2022 • edited Loading

❌ Deploy Preview for netlify-plugin-nextjs-next-auth-demo failed.

netlify bot commented Nov 15, 2022 • edited Loading

❌ Deploy Preview for netlify-plugin-nextjs-export-demo failed.

netlify bot commented Nov 15, 2022 • edited Loading

❌ Deploy Preview for next-plugin-canary failed.

netlify bot commented Nov 15, 2022 • edited Loading

✅ Deploy Preview for next-i18next-demo ready!

netlify bot commented Nov 15, 2022 • edited Loading

❌ Deploy Preview for nextjs-plugin-next-11-demo failed.

netlify bot commented Nov 15, 2022 • edited Loading

✅ Deploy Preview for netlify-plugin-nextjs-demo ready!

nickytonline left a comment

Choose a reason for hiding this comment

nickytonline commented Nov 18, 2022

sarahetter Nov 18, 2022

Choose a reason for hiding this comment

sarahetter left a comment

Choose a reason for hiding this comment

nickytonline commented Nov 18, 2022

nickytonline commented Nov 18, 2022

ascorbic commented Nov 18, 2022

nickytonline commented Nov 18, 2022 • edited Loading

nickytonline commented Nov 15, 2022 •

edited

Loading

netlify bot commented Nov 15, 2022 •

edited

Loading

netlify bot commented Nov 15, 2022 •

edited

Loading

netlify bot commented Nov 15, 2022 •

edited

Loading

netlify bot commented Nov 15, 2022 •

edited

Loading

netlify bot commented Nov 15, 2022 •

edited

Loading

netlify bot commented Nov 15, 2022 •

edited

Loading

netlify bot commented Nov 15, 2022 •

edited

Loading

netlify bot commented Nov 15, 2022 •

edited

Loading

netlify bot commented Nov 15, 2022 •

edited

Loading

netlify bot commented Nov 15, 2022 •

edited

Loading

nickytonline commented Nov 18, 2022 •

edited

Loading