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.

Sign up for GitHub

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

Jump to bottom

Use useRouter over Router for with-google-analytics example #16846

Merged
merged 2 commits into from
Sep 4, 2020
Merged

Use useRouter over Router for with-google-analytics example #16846

merged 2 commits into from
Sep 4, 2020

Conversation

chibicode
Copy link
Member

Updating with-google-analytics example to be in line with our documentation recommending useRouter. Verified that it works.

@ijjk ijjk added the examples Issue/PR related to examples label Sep 4, 2020
@ijjk
Copy link
Member

ijjk commented Sep 4, 2020

Stats from current PR

Default Server Mode (Increase detected ⚠️)
General
vercel/next.js canary chibicode/next.js hotfix-router-with-google-analytics Change
buildDuration 12.5s 12.7s ⚠️ +206ms
nodeModulesSize 56.7 MB 56.7 MB ✓
Page Load Tests Overall increase ✓
vercel/next.js canary chibicode/next.js hotfix-router-with-google-analytics Change
/ failed reqs 0 0 ✓
/ total time (seconds) 2.464 2.386 -0.08
/ avg req/sec 1014.6 1047.78 +33.18
/error-in-render failed reqs 0 0 ✓
/error-in-render total time (seconds) 1.426 1.373 -0.05
/error-in-render avg req/sec 1753.38 1821.26 +67.88
Client Bundles (main, webpack, commons)
vercel/next.js canary chibicode/next.js hotfix-router-with-google-analytics Change
677f882d2ed8..b5de.js gzip 10.5 kB 10.5 kB ✓
framework.HASH.js gzip 39 kB 39 kB ✓
main-ec49a3f..d437.js gzip 7.08 kB 7.08 kB ✓
webpack-e067..f178.js gzip 751 B 751 B ✓
Overall change 57.3 kB 57.3 kB ✓
Client Bundles (main, webpack, commons) Modern
vercel/next.js canary chibicode/next.js hotfix-router-with-google-analytics Change
677f882d2ed8..dule.js gzip 6.4 kB 6.4 kB ✓
framework.HA..dule.js gzip 39 kB 39 kB ✓
main-b3aecf0..dule.js gzip 6.14 kB 6.14 kB ✓
webpack-07c5..dule.js gzip 751 B 751 B ✓
Overall change 52.3 kB 52.3 kB ✓
Legacy Client Bundles (polyfills)
vercel/next.js canary chibicode/next.js hotfix-router-with-google-analytics Change
polyfills-4b..e242.js gzip 31 kB 31 kB ✓
Overall change 31 kB 31 kB ✓
Client Pages
vercel/next.js canary chibicode/next.js hotfix-router-with-google-analytics Change
_app-9a0b9e1..b37e.js gzip 1.28 kB 1.28 kB ✓
_error-ed1b0..8fbd.js gzip 3.44 kB 3.44 kB ✓
hooks-89731c..c609.js gzip 887 B 887 B ✓
index-17468f..5d83.js gzip 227 B 227 B ✓
link-d2344ce..8b36.js gzip 1.3 kB 1.3 kB ✓
routerDirect..924c.js gzip 284 B 284 B ✓
withRouter-7..c13d.js gzip 284 B 284 B ✓
Overall change 7.71 kB 7.71 kB ✓
Client Pages Modern
vercel/next.js canary chibicode/next.js hotfix-router-with-google-analytics Change
_app-75d3a82..dule.js gzip 625 B 625 B ✓
_error-4469a..dule.js gzip 2.29 kB 2.29 kB ✓
hooks-cbf13f..dule.js gzip 387 B 387 B ✓
index-b9a643..dule.js gzip 226 B 226 B ✓
link-f8c0daf..dule.js gzip 1.26 kB 1.26 kB ✓
routerDirect..dule.js gzip 284 B 284 B ✓
withRouter-f..dule.js gzip 282 B 282 B ✓
Overall change 5.36 kB 5.36 kB ✓
Client Build Manifests
vercel/next.js canary chibicode/next.js hotfix-router-with-google-analytics Change
_buildManifest.js gzip 322 B 322 B ✓
_buildManife..dule.js gzip 329 B 329 B ✓
Overall change 651 B 651 B ✓
Rendered Page Sizes
vercel/next.js canary chibicode/next.js hotfix-router-with-google-analytics Change
index.html gzip 971 B 971 B ✓
link.html gzip 976 B 976 B ✓
withRouter.html gzip 963 B 963 B ✓
Overall change 2.91 kB 2.91 kB ✓
Serverless Mode
General
vercel/next.js canary chibicode/next.js hotfix-router-with-google-analytics Change
buildDuration 14.1s 14.5s ⚠️ +374ms
nodeModulesSize 56.7 MB 56.7 MB ✓
Client Bundles (main, webpack, commons)
vercel/next.js canary chibicode/next.js hotfix-router-with-google-analytics Change
677f882d2ed8..b5de.js gzip 10.5 kB 10.5 kB ✓
framework.HASH.js gzip 39 kB 39 kB ✓
main-ec49a3f..d437.js gzip 7.08 kB 7.08 kB ✓
webpack-e067..f178.js gzip 751 B 751 B ✓
Overall change 57.3 kB 57.3 kB ✓
Client Bundles (main, webpack, commons) Modern
vercel/next.js canary chibicode/next.js hotfix-router-with-google-analytics Change
677f882d2ed8..dule.js gzip 6.4 kB 6.4 kB ✓
framework.HA..dule.js gzip 39 kB 39 kB ✓
main-b3aecf0..dule.js gzip 6.14 kB 6.14 kB ✓
webpack-07c5..dule.js gzip 751 B 751 B ✓
Overall change 52.3 kB 52.3 kB ✓
Legacy Client Bundles (polyfills)
vercel/next.js canary chibicode/next.js hotfix-router-with-google-analytics Change
polyfills-4b..e242.js gzip 31 kB 31 kB ✓
Overall change 31 kB 31 kB ✓
Client Pages
vercel/next.js canary chibicode/next.js hotfix-router-with-google-analytics Change
_app-9a0b9e1..b37e.js gzip 1.28 kB 1.28 kB ✓
_error-ed1b0..8fbd.js gzip 3.44 kB 3.44 kB ✓
hooks-89731c..c609.js gzip 887 B 887 B ✓
index-17468f..5d83.js gzip 227 B 227 B ✓
link-d2344ce..8b36.js gzip 1.3 kB 1.3 kB ✓
routerDirect..924c.js gzip 284 B 284 B ✓
withRouter-7..c13d.js gzip 284 B 284 B ✓
Overall change 7.71 kB 7.71 kB ✓
Client Pages Modern
vercel/next.js canary chibicode/next.js hotfix-router-with-google-analytics Change
_app-75d3a82..dule.js gzip 625 B 625 B ✓
_error-4469a..dule.js gzip 2.29 kB 2.29 kB ✓
hooks-cbf13f..dule.js gzip 387 B 387 B ✓
index-b9a643..dule.js gzip 226 B 226 B ✓
link-f8c0daf..dule.js gzip 1.26 kB 1.26 kB ✓
routerDirect..dule.js gzip 284 B 284 B ✓
withRouter-f..dule.js gzip 282 B 282 B ✓
Overall change 5.36 kB 5.36 kB ✓
Client Build Manifests
vercel/next.js canary chibicode/next.js hotfix-router-with-google-analytics Change
_buildManifest.js gzip 322 B 322 B ✓
_buildManife..dule.js gzip 329 B 329 B ✓
Overall change 651 B 651 B ✓
Serverless bundles
vercel/next.js canary chibicode/next.js hotfix-router-with-google-analytics Change
_error.js 1.03 MB 1.03 MB ✓
404.html 4.22 kB 4.22 kB ✓
hooks.html 3.86 kB 3.86 kB ✓
index.js 1.03 MB 1.03 MB ✓
link.js 1.08 MB 1.08 MB ✓
routerDirect.js 1.07 MB 1.07 MB ✓
withRouter.js 1.07 MB 1.07 MB ✓
Overall change 5.29 MB 5.29 MB ✓
Commit: 955e6f3

chibicode
chibicode commented Sep 4, 2020
View reviewed changes
examples/with-google-analytics/pages/_app.js
}
}, [])
}, [router.events])
Copy link
Member Author

Choose a reason for hiding this comment

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

router.events was added with exhaustive-deps. I think this is fine?

Copy link
Member

Choose a reason for hiding this comment

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

Yes

lfades
lfades approved these changes Sep 4, 2020
View reviewed changes
Copy link
Member

@lfades lfades left a comment

Choose a reason for hiding this comment

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

Thank you!

@ijjk
Copy link
Member

ijjk commented Sep 4, 2020

Stats from current PR

Default Server Mode (Decrease detected ✓)
General
vercel/next.js canary chibicode/next.js hotfix-router-with-google-analytics Change
buildDuration 13.7s 14s ⚠️ +283ms
nodeModulesSize 56.7 MB 56.7 MB ✓
Page Load Tests Overall decrease ⚠️
vercel/next.js canary chibicode/next.js hotfix-router-with-google-analytics Change
/ failed reqs 0 0 ✓
/ total time (seconds) 2.605 2.585 -0.02
/ avg req/sec 959.57 967.28 +7.71
/error-in-render failed reqs 0 0 ✓
/error-in-render total time (seconds) 1.532 1.606 ⚠️ +0.07
/error-in-render avg req/sec 1631.38 1556.45 ⚠️ -74.93
Client Bundles (main, webpack, commons)
vercel/next.js canary chibicode/next.js hotfix-router-with-google-analytics Change
677f882d2ed8..b5de.js gzip 10.5 kB 10.5 kB ✓
framework.HASH.js gzip 39 kB 39 kB ✓
main-195c642..b6b7.js gzip 7.08 kB 7.08 kB ✓
webpack-e067..f178.js gzip 751 B 751 B ✓
Overall change 57.3 kB 57.3 kB ✓
Client Bundles (main, webpack, commons) Modern
vercel/next.js canary chibicode/next.js hotfix-router-with-google-analytics Change
677f882d2ed8..dule.js gzip 6.4 kB 6.4 kB ✓
framework.HA..dule.js gzip 39 kB 39 kB ✓
main-ad445e5..dule.js gzip 6.14 kB 6.14 kB ✓
webpack-07c5..dule.js gzip 751 B 751 B ✓
Overall change 52.3 kB 52.3 kB ✓
Legacy Client Bundles (polyfills)
vercel/next.js canary chibicode/next.js hotfix-router-with-google-analytics Change
polyfills-4b..e242.js gzip 31 kB 31 kB ✓
Overall change 31 kB 31 kB ✓
Client Pages
vercel/next.js canary chibicode/next.js hotfix-router-with-google-analytics Change
_app-9a0b9e1..b37e.js gzip 1.28 kB 1.28 kB ✓
_error-ed1b0..8fbd.js gzip 3.44 kB 3.44 kB ✓
hooks-89731c..c609.js gzip 887 B 887 B ✓
index-17468f..5d83.js gzip 227 B 227 B ✓
link-d2344ce..8b36.js gzip 1.3 kB 1.3 kB ✓
routerDirect..924c.js gzip 284 B 284 B ✓
withRouter-7..c13d.js gzip 284 B 284 B ✓
Overall change 7.71 kB 7.71 kB ✓
Client Pages Modern
vercel/next.js canary chibicode/next.js hotfix-router-with-google-analytics Change
_app-75d3a82..dule.js gzip 625 B 625 B ✓
_error-4469a..dule.js gzip 2.29 kB 2.29 kB ✓
hooks-cbf13f..dule.js gzip 387 B 387 B ✓
index-b9a643..dule.js gzip 226 B 226 B ✓
link-f8c0daf..dule.js gzip 1.26 kB 1.26 kB ✓
routerDirect..dule.js gzip 284 B 284 B ✓
withRouter-f..dule.js gzip 282 B 282 B ✓
Overall change 5.36 kB 5.36 kB ✓
Client Build Manifests
vercel/next.js canary chibicode/next.js hotfix-router-with-google-analytics Change
_buildManifest.js gzip 322 B 322 B ✓
_buildManife..dule.js gzip 329 B 329 B ✓
Overall change 651 B 651 B ✓
Rendered Page Sizes
vercel/next.js canary chibicode/next.js hotfix-router-with-google-analytics Change
index.html gzip 968 B 968 B ✓
link.html gzip 975 B 975 B ✓
withRouter.html gzip 962 B 962 B ✓
Overall change 2.9 kB 2.9 kB ✓
Serverless Mode
General
vercel/next.js canary chibicode/next.js hotfix-router-with-google-analytics Change
buildDuration 16s 15.4s -607ms
nodeModulesSize 56.7 MB 56.7 MB ✓
Client Bundles (main, webpack, commons)
vercel/next.js canary chibicode/next.js hotfix-router-with-google-analytics Change
677f882d2ed8..b5de.js gzip 10.5 kB 10.5 kB ✓
framework.HASH.js gzip 39 kB 39 kB ✓
main-195c642..b6b7.js gzip 7.08 kB 7.08 kB ✓
webpack-e067..f178.js gzip 751 B 751 B ✓
Overall change 57.3 kB 57.3 kB ✓
Client Bundles (main, webpack, commons) Modern
vercel/next.js canary chibicode/next.js hotfix-router-with-google-analytics Change
677f882d2ed8..dule.js gzip 6.4 kB 6.4 kB ✓
framework.HA..dule.js gzip 39 kB 39 kB ✓
main-ad445e5..dule.js gzip 6.14 kB 6.14 kB ✓
webpack-07c5..dule.js gzip 751 B 751 B ✓
Overall change 52.3 kB 52.3 kB ✓
Legacy Client Bundles (polyfills)
vercel/next.js canary chibicode/next.js hotfix-router-with-google-analytics Change
polyfills-4b..e242.js gzip 31 kB 31 kB ✓
Overall change 31 kB 31 kB ✓
Client Pages
vercel/next.js canary chibicode/next.js hotfix-router-with-google-analytics Change
_app-9a0b9e1..b37e.js gzip 1.28 kB 1.28 kB ✓
_error-ed1b0..8fbd.js gzip 3.44 kB 3.44 kB ✓
hooks-89731c..c609.js gzip 887 B 887 B ✓
index-17468f..5d83.js gzip 227 B 227 B ✓
link-d2344ce..8b36.js gzip 1.3 kB 1.3 kB ✓
routerDirect..924c.js gzip 284 B 284 B ✓
withRouter-7..c13d.js gzip 284 B 284 B ✓
Overall change 7.71 kB 7.71 kB ✓
Client Pages Modern
vercel/next.js canary chibicode/next.js hotfix-router-with-google-analytics Change
_app-75d3a82..dule.js gzip 625 B 625 B ✓
_error-4469a..dule.js gzip 2.29 kB 2.29 kB ✓
hooks-cbf13f..dule.js gzip 387 B 387 B ✓
index-b9a643..dule.js gzip 226 B 226 B ✓
link-f8c0daf..dule.js gzip 1.26 kB 1.26 kB ✓
routerDirect..dule.js gzip 284 B 284 B ✓
withRouter-f..dule.js gzip 282 B 282 B ✓
Overall change 5.36 kB 5.36 kB ✓
Client Build Manifests
vercel/next.js canary chibicode/next.js hotfix-router-with-google-analytics Change
_buildManifest.js gzip 322 B 322 B ✓
_buildManife..dule.js gzip 329 B 329 B ✓
Overall change 651 B 651 B ✓
Serverless bundles
vercel/next.js canary chibicode/next.js hotfix-router-with-google-analytics Change
_error.js 1.03 MB 1.03 MB ✓
404.html 4.22 kB 4.22 kB ✓
hooks.html 3.86 kB 3.86 kB ✓
index.js 1.03 MB 1.03 MB ✓
link.js 1.08 MB 1.08 MB ✓
routerDirect.js 1.07 MB 1.07 MB ✓
withRouter.js 1.07 MB 1.07 MB ✓
Overall change 5.29 MB 5.29 MB ✓
Commit: b6b1ed6

@kodiakhq kodiakhq bot merged commit 1c7cc60 into vercel:canary Sep 4, 2020
@balazsorban44
Copy link
Member

@chibicode what's the gain here?

@lfades
Copy link
Member

lfades commented Sep 5, 2020 •
edited
Loading

@balazsorban44 We no longer document or recommend the usage of importing the Router object over using the useRouter hook.

@balazsorban44
Copy link
Member

@lfades Yeah, I understand that, but is there a reason, like maybe removing the Router import possibility I in the future? Or any performance gains? Or is it simply not the "React way"?

Just curious/trying to understand. :)

@lfades
Copy link
Member

lfades commented Sep 5, 2020

@balazsorban44 With useRouter you have to inside of a component in order to use the router and we would like to keep it that way, using the router outside of a component will lead you to unexpected results.

@chibicode chibicode deleted the hotfix-router-with-google-analytics branch September 5, 2020 20:08
HitoriSensei pushed a commit to HitoriSensei/next.js that referenced this pull request Sep 26, 2020
@chibicode
Use useRouter over Router for with-google-analytics example (vercel#1…
4def0a2 
…6846)

Updating `with-google-analytics` example to be in line with [our documentation recommending `useRouter`](https://nextjs.org/docs/api-reference/next/router#userouter). Verified that it works.
@ccorda ccorda mentioned this pull request Jul 21, 2021
Closed
@vercel vercel locked as resolved and limited conversation to collaborators Jan 29, 2022
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Reviewers

@ijjk ijjk ijjk approved these changes

@lfades lfades lfades approved these changes

@Timer Timer Awaiting requested review from Timer

@timneutkens timneutkens Awaiting requested review from timneutkens

Assignees
No one assigned
Labels
examples Issue/PR related to examples
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
4 participants
@chibicode @ijjk @balazsorban44 @lfades