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

Add test for title and routeChangeComplete #14132

Merged
merged 2 commits into from
Jun 12, 2020
Merged

Add test for title and routeChangeComplete #14132

merged 2 commits into from
Jun 12, 2020

Conversation

ijjk
Copy link
Member

@ijjk ijjk commented Jun 12, 2020

This adds a test case for the document.title not being updated by the time routeChangeComplete is fired. This should have been made consistent with #13287 so should be able to be closed now

Closes: #6025

@vercel
Copy link

vercel bot commented Jun 12, 2020

This pull request is being automatically deployed with Vercel (learn more).
To see the status of your deployment, click on the icon next to each commit.

Timer
Timer approved these changes Jun 12, 2020
View reviewed changes
Copy link
Member

@Timer Timer left a comment

Choose a reason for hiding this comment

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

LGTM if test passes.

@ijjk
Copy link
Member Author

ijjk commented Jun 12, 2020

Stats from current PR

Default Server Mode (Decrease detected ✓)
General
vercel/next.js canary ijjk/next.js test/route-change-title Change
buildDuration 13.2s 13.3s ⚠️ +115ms
nodeModulesSize 65.2 MB 65.2 MB ✓
Page Load Tests Overall decrease ⚠️
vercel/next.js canary ijjk/next.js test/route-change-title Change
/ failed reqs 0 0 ✓
/ total time (seconds) 2.365 2.369 0
/ avg req/sec 1057.17 1055.33 -1.84
/error-in-render failed reqs 0 0 ✓
/error-in-render total time (seconds) 1.497 1.507 ⚠️ +0.01
/error-in-render avg req/sec 1669.89 1659.46 -10.43
Client Bundles (main, webpack, commons)
vercel/next.js canary ijjk/next.js test/route-change-title Change
main-HASH.js gzip 6.51 kB 6.51 kB ✓
webpack-HASH.js gzip 746 B 746 B ✓
de003c3a9d30..0538.js gzip 10.5 kB 10.5 kB ✓
framework.HASH.js gzip 39.1 kB 39.1 kB ✓
Overall change 56.9 kB 56.9 kB ✓
Client Bundles (main, webpack, commons) Modern
vercel/next.js canary ijjk/next.js test/route-change-title Change
main-HASH.module.js gzip 5.6 kB 5.6 kB ✓
webpack-HASH..dule.js gzip 746 B 746 B ✓
de003c3a9d30..dule.js gzip 6.93 kB 6.93 kB ✓
framework.HA..dule.js gzip 39.1 kB 39.1 kB ✓
Overall change 52.4 kB 52.4 kB ✓
Legacy Client Bundles (polyfills)
vercel/next.js canary ijjk/next.js test/route-change-title Change
polyfills-HASH.js gzip 26.3 kB 26.3 kB ✓
Overall change 26.3 kB 26.3 kB ✓
Client Pages
vercel/next.js canary ijjk/next.js test/route-change-title Change
routerDirect.js gzip 279 B 279 B ✓
withRouter.js gzip 278 B 278 B ✓
_error.js gzip 3.37 kB 3.37 kB ✓
index.js gzip 222 B 222 B ✓
link.js gzip 2.05 kB 2.05 kB ✓
hooks.js gzip 881 B 881 B ✓
_app.js gzip 1.26 kB 1.26 kB ✓
Overall change 8.34 kB 8.34 kB ✓
Client Pages Modern
vercel/next.js canary ijjk/next.js test/route-change-title Change
index.module.js gzip 223 B 223 B ✓
routerDirect..dule.js gzip 281 B 281 B ✓
withRouter.m..dule.js gzip 278 B 278 B ✓
hooks.module.js gzip 383 B 383 B ✓
_error.module.js gzip 2.21 kB 2.21 kB ✓
link.module.js gzip 1.52 kB 1.52 kB ✓
_app.module.js gzip 604 B 604 B ✓
Overall change 5.49 kB 5.49 kB ✓
Client Build Manifests
vercel/next.js canary ijjk/next.js test/route-change-title Change
_buildManifest.js gzip 270 B 270 B ✓
_buildManife..dule.js gzip 274 B 274 B ✓
Overall change 544 B 544 B ✓
Rendered Page Sizes
vercel/next.js canary ijjk/next.js test/route-change-title Change
index.html gzip 956 B 956 B ✓
link.html gzip 963 B 963 B ✓
withRouter.html gzip 949 B 949 B ✓
Overall change 2.87 kB 2.87 kB ✓
Serverless Mode
General
vercel/next.js canary ijjk/next.js test/route-change-title Change
buildDuration 14.9s 14.4s -426ms
nodeModulesSize 65.2 MB 65.2 MB ✓
Client Bundles (main, webpack, commons)
vercel/next.js canary ijjk/next.js test/route-change-title Change
main-HASH.js gzip 6.51 kB 6.51 kB ✓
webpack-HASH.js gzip 746 B 746 B ✓
de003c3a9d30..0538.js gzip 10.5 kB 10.5 kB ✓
framework.HASH.js gzip 39.1 kB 39.1 kB ✓
Overall change 56.9 kB 56.9 kB ✓
Client Bundles (main, webpack, commons) Modern
vercel/next.js canary ijjk/next.js test/route-change-title Change
main-HASH.module.js gzip 5.6 kB 5.6 kB ✓
webpack-HASH..dule.js gzip 746 B 746 B ✓
de003c3a9d30..dule.js gzip 6.93 kB 6.93 kB ✓
framework.HA..dule.js gzip 39.1 kB 39.1 kB ✓
Overall change 52.4 kB 52.4 kB ✓
Legacy Client Bundles (polyfills)
vercel/next.js canary ijjk/next.js test/route-change-title Change
polyfills-HASH.js gzip 26.3 kB 26.3 kB ✓
Overall change 26.3 kB 26.3 kB ✓
Client Pages
vercel/next.js canary ijjk/next.js test/route-change-title Change
routerDirect.js gzip 279 B 279 B ✓
withRouter.js gzip 278 B 278 B ✓
_error.js gzip 3.37 kB 3.37 kB ✓
index.js gzip 222 B 222 B ✓
link.js gzip 2.05 kB 2.05 kB ✓
hooks.js gzip 881 B 881 B ✓
_app.js gzip 1.26 kB 1.26 kB ✓
Overall change 8.34 kB 8.34 kB ✓
Client Pages Modern
vercel/next.js canary ijjk/next.js test/route-change-title Change
index.module.js gzip 223 B 223 B ✓
routerDirect..dule.js gzip 281 B 281 B ✓
withRouter.m..dule.js gzip 278 B 278 B ✓
hooks.module.js gzip 383 B 383 B ✓
_error.module.js gzip 2.21 kB 2.21 kB ✓
link.module.js gzip 1.52 kB 1.52 kB ✓
_app.module.js gzip 604 B 604 B ✓
Overall change 5.49 kB 5.49 kB ✓
Client Build Manifests
vercel/next.js canary ijjk/next.js test/route-change-title Change
_buildManifest.js gzip 270 B 270 B ✓
_buildManife..dule.js gzip 274 B 274 B ✓
Overall change 544 B 544 B ✓
Serverless bundles
vercel/next.js canary ijjk/next.js test/route-change-title Change
_error.js 875 kB 875 kB ✓
404.html 4.17 kB 4.17 kB ✓
hooks.html 3.79 kB 3.79 kB ✓
index.js 875 kB 875 kB ✓
link.js 913 kB 913 kB ✓
routerDirect.js 905 kB 905 kB ✓
withRouter.js 905 kB 905 kB ✓
Overall change 4.48 MB 4.48 MB ✓

timneutkens
timneutkens previously approved these changes Jun 12, 2020
View reviewed changes
Timer
Timer requested changes Jun 12, 2020
View reviewed changes
Copy link
Member

@Timer Timer left a comment

Choose a reason for hiding this comment

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

Failed in IE11:

Production Usage › With navigation › should set title by routeChangeComplete event

    thrown: "Exceeded timeout of 300000 ms for a test.
    Use jest.setTimeout(newTimeout) to increase the timeout value, if this is a long-running test."

      323 |     })
      324 | 
    > 325 |     it('should set title by routeChangeComplete event', async () => {
          |     ^
      326 |       const browser = await webdriver(appPort, '/')
      327 |       await browser.eval(() => {
      328 |         window.next.router.events.on('routeChangeComplete', (url) => {

      at describe (integration/production/test/index.test.js:325:5)
      at Object.<anonymous>.describe (integration/production/test/index.test.js:298:3)
      at Object.<anonymous> (integration/production/test/index.test.js:34:1)

Test Suites: 1 failed, 1 total
Tests:       1 failed, 82 passed, 83 total
Snapshots:   0 total

@ijjk
Copy link
Member Author

ijjk commented Jun 12, 2020

Updated to ensure the eval calls in the test are compatible with ie11

@ijjk ijjk requested a review from Timer June 12, 2020 16:06
@ijjk
Copy link
Member Author

ijjk commented Jun 12, 2020

Stats from current PR

Default Server Mode (Increase detected ⚠️)
General
vercel/next.js canary ijjk/next.js test/route-change-title Change
buildDuration 13s 13s ⚠️ +17ms
nodeModulesSize 65.2 MB 65.2 MB ✓
Page Load Tests Overall increase ✓
vercel/next.js canary ijjk/next.js test/route-change-title Change
/ failed reqs 0 0 ✓
/ total time (seconds) 2.295 2.29 0
/ avg req/sec 1089.21 1091.69 ⚠️ +2.48
/error-in-render failed reqs 0 0 ✓
/error-in-render total time (seconds) 1.57 1.45 -0.12
/error-in-render avg req/sec 1592.68 1724.54 ⚠️ +131.86
Client Bundles (main, webpack, commons)
vercel/next.js canary ijjk/next.js test/route-change-title Change
main-HASH.js gzip 6.51 kB 6.51 kB ✓
webpack-HASH.js gzip 746 B 746 B ✓
de003c3a9d30..0538.js gzip 10.5 kB 10.5 kB ✓
framework.HASH.js gzip 39.1 kB 39.1 kB ✓
Overall change 56.9 kB 56.9 kB ✓
Client Bundles (main, webpack, commons) Modern
vercel/next.js canary ijjk/next.js test/route-change-title Change
main-HASH.module.js gzip 5.6 kB 5.6 kB ✓
webpack-HASH..dule.js gzip 746 B 746 B ✓
de003c3a9d30..dule.js gzip 6.93 kB 6.93 kB ✓
framework.HA..dule.js gzip 39.1 kB 39.1 kB ✓
Overall change 52.4 kB 52.4 kB ✓
Legacy Client Bundles (polyfills)
vercel/next.js canary ijjk/next.js test/route-change-title Change
polyfills-HASH.js gzip 26.3 kB 26.3 kB ✓
Overall change 26.3 kB 26.3 kB ✓
Client Pages
vercel/next.js canary ijjk/next.js test/route-change-title Change
routerDirect.js gzip 279 B 279 B ✓
withRouter.js gzip 278 B 278 B ✓
_error.js gzip 3.37 kB 3.37 kB ✓
index.js gzip 222 B 222 B ✓
link.js gzip 2.05 kB 2.05 kB ✓
hooks.js gzip 881 B 881 B ✓
_app.js gzip 1.26 kB 1.26 kB ✓
Overall change 8.34 kB 8.34 kB ✓
Client Pages Modern
vercel/next.js canary ijjk/next.js test/route-change-title Change
index.module.js gzip 223 B 223 B ✓
routerDirect..dule.js gzip 281 B 281 B ✓
withRouter.m..dule.js gzip 278 B 278 B ✓
hooks.module.js gzip 383 B 383 B ✓
_error.module.js gzip 2.21 kB 2.21 kB ✓
link.module.js gzip 1.52 kB 1.52 kB ✓
_app.module.js gzip 604 B 604 B ✓
Overall change 5.49 kB 5.49 kB ✓
Client Build Manifests
vercel/next.js canary ijjk/next.js test/route-change-title Change
_buildManifest.js gzip 270 B 270 B ✓
_buildManife..dule.js gzip 274 B 274 B ✓
Overall change 544 B 544 B ✓
Rendered Page Sizes
vercel/next.js canary ijjk/next.js test/route-change-title Change
index.html gzip 956 B 956 B ✓
link.html gzip 963 B 963 B ✓
withRouter.html gzip 949 B 949 B ✓
Overall change 2.87 kB 2.87 kB ✓
Serverless Mode
General
vercel/next.js canary ijjk/next.js test/route-change-title Change
buildDuration 14.4s 14.3s -48ms
nodeModulesSize 65.2 MB 65.2 MB ✓
Client Bundles (main, webpack, commons)
vercel/next.js canary ijjk/next.js test/route-change-title Change
main-HASH.js gzip 6.51 kB 6.51 kB ✓
webpack-HASH.js gzip 746 B 746 B ✓
de003c3a9d30..0538.js gzip 10.5 kB 10.5 kB ✓
framework.HASH.js gzip 39.1 kB 39.1 kB ✓
Overall change 56.9 kB 56.9 kB ✓
Client Bundles (main, webpack, commons) Modern
vercel/next.js canary ijjk/next.js test/route-change-title Change
main-HASH.module.js gzip 5.6 kB 5.6 kB ✓
webpack-HASH..dule.js gzip 746 B 746 B ✓
de003c3a9d30..dule.js gzip 6.93 kB 6.93 kB ✓
framework.HA..dule.js gzip 39.1 kB 39.1 kB ✓
Overall change 52.4 kB 52.4 kB ✓
Legacy Client Bundles (polyfills)
vercel/next.js canary ijjk/next.js test/route-change-title Change
polyfills-HASH.js gzip 26.3 kB 26.3 kB ✓
Overall change 26.3 kB 26.3 kB ✓
Client Pages
vercel/next.js canary ijjk/next.js test/route-change-title Change
routerDirect.js gzip 279 B 279 B ✓
withRouter.js gzip 278 B 278 B ✓
_error.js gzip 3.37 kB 3.37 kB ✓
index.js gzip 222 B 222 B ✓
link.js gzip 2.05 kB 2.05 kB ✓
hooks.js gzip 881 B 881 B ✓
_app.js gzip 1.26 kB 1.26 kB ✓
Overall change 8.34 kB 8.34 kB ✓
Client Pages Modern
vercel/next.js canary ijjk/next.js test/route-change-title Change
index.module.js gzip 223 B 223 B ✓
routerDirect..dule.js gzip 281 B 281 B ✓
withRouter.m..dule.js gzip 278 B 278 B ✓
hooks.module.js gzip 383 B 383 B ✓
_error.module.js gzip 2.21 kB 2.21 kB ✓
link.module.js gzip 1.52 kB 1.52 kB ✓
_app.module.js gzip 604 B 604 B ✓
Overall change 5.49 kB 5.49 kB ✓
Client Build Manifests
vercel/next.js canary ijjk/next.js test/route-change-title Change
_buildManifest.js gzip 270 B 270 B ✓
_buildManife..dule.js gzip 274 B 274 B ✓
Overall change 544 B 544 B ✓
Serverless bundles
vercel/next.js canary ijjk/next.js test/route-change-title Change
_error.js 875 kB 875 kB ✓
404.html 4.17 kB 4.17 kB ✓
hooks.html 3.79 kB 3.79 kB ✓
index.js 875 kB 875 kB ✓
link.js 913 kB 913 kB ✓
routerDirect.js 905 kB 905 kB ✓
withRouter.js 905 kB 905 kB ✓
Overall change 4.48 MB 4.48 MB ✓

@kodiakhq kodiakhq bot merged commit 0a02efb into vercel:canary Jun 12, 2020
rokinsky pushed a commit to rokinsky/next.js that referenced this pull request Jul 11, 2020
@ijjk @rokinsky
Add test for title and routeChangeComplete (vercel#14132)
d9c5321 
This adds a test case for the `document.title` not being updated by the time `routeChangeComplete` is fired. This should have been made consistent with vercel#13287 so should be able to be closed now

Closes: vercel#6025
@hustKiwi hustKiwi mentioned this pull request Aug 15, 2021
Merged
@vercel vercel locked as resolved and limited conversation to collaborators Jan 30, 2022
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Reviewers

@Timer Timer Timer approved these changes

@timneutkens timneutkens timneutkens left review comments

@lfades lfades Awaiting requested review from lfades

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

<Head><title> not set by the time history change event fires
3 participants
@ijjk @Timer @timneutkens