Skip to content

Remove old UNSAFE_ lifecycle methods #5020

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

Merged
merged 5 commits into from
Aug 25, 2018
Merged

Remove old UNSAFE_ lifecycle methods #5020

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
5 commits
Select commit Hold shift + click to select a range
0a86fe6
remove old UNSAFE_ lifecycle methods
mfix22 Aug 23, 2018
249371a
Merge branch 'canary' into remove-unsafe-methods
Aug 23, 2018
5167828
add integration test for client routing with next/head updates
mfix22 Aug 24, 2018
c0de785
fix client navigation head update test
mfix22 Aug 25, 2018
602d390
Make test close the browser + move into nav folder
timneutkens Aug 25, 2018
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
16 changes: 6 additions & 10 deletions lib/side-effect.js
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,5 +1,4 @@
import React, { Component } from 'react'
import {polyfill} from 'react-lifecycles-compat'
import { getDisplayName } from './utils'

export default function withSideEffect (reduceComponentsToState, handleStateChangeOnClient, mapStateOnServer) {
Expand Down Expand Up @@ -57,18 +56,18 @@ export default function withSideEffect (reduceComponentsToState, handleStateChan
return recordedState
}

componentDidUpdate () {
constructor (props) {
super(props)
mountedInstances.add(this)
emitChange(this)
}

componentWillUnmount () {
mountedInstances.delete(this)
componentDidUpdate () {
emitChange(this)
}

// eslint-disable-next-line camelcase
UNSAFE_componentWillMount () {
mountedInstances.add(this)
componentWillUnmount () {
mountedInstances.delete(this)
emitChange(this)
}

Expand All @@ -77,9 +76,6 @@ export default function withSideEffect (reduceComponentsToState, handleStateChan
}
}

// Make UNSAFE_ compatible with version of React under 16.3
polyfill(SideEffect)
Copy link
Author

@mfix22 mfix22 Aug 23, 2018 •
edited
Loading

Choose a reason for hiding this comment

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

@timneutkens @giuseppeg I see that a couple tests still use UNSAFE_ . . . do you want me to keep react-lifecycles-compat

Copy link
Member

Choose a reason for hiding this comment

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

Tests don't need the compat because we run tests on 16.4

Copy link
Member

Choose a reason for hiding this comment

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

So removing it is perfect 👍


return SideEffect
}
}
1 change: 0 additions & 1 deletion package.json
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -94,7 +94,6 @@
"prop-types": "15.6.0",
"prop-types-exact": "1.1.1",
"react-error-overlay": "4.0.0",
"react-lifecycles-compat": "3.0.4",
"react-loadable": "5.4.0",
"recursive-copy": "2.0.6",
"resolve": "1.5.0",
Expand Down
15 changes: 15 additions & 0 deletions test/integration/basic/pages/nav/head-1.js
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,15 @@
import React from 'react'
import Head from 'next/head'
import Link from 'next/link'

export default (props) => <div id='head-1'>
<Head>
<meta
name='description'
content='Head One'
/>
</Head>
<Link href='/nav/head-2'>
<a id='to-head-2'>to head 2</a>
</Link>
</div>
15 changes: 15 additions & 0 deletions test/integration/basic/pages/nav/head-2.js
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,15 @@
import React from 'react'
import Head from 'next/head'
import Link from 'next/link'

export default (props) => <div id='head-2'>
<Head>
<meta
name='description'
content='Head Two'
/>
</Head>
<Link href='/nav/head-1'>
<a id='to-head-1'>to head 1</a>
</Link>
</div>
18 changes: 18 additions & 0 deletions test/integration/basic/test/client-navigation.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -692,5 +692,23 @@ export default (context, render) => {
browser.close()
})
})

describe('updating head while client routing', () => {
it('should update head during client routing', async () => {
let browser
try {
browser = await webdriver(context.appPort, '/nav/head-1')
expect(await browser.elementByCss('meta[name="description"]').getAttribute('content')).toBe('Head One')
await browser.elementByCss('#to-head-2').click().waitForElementByCss('#head-2')
expect(await browser.elementByCss('meta[name="description"]').getAttribute('content')).toBe('Head Two')
await browser.elementByCss('#to-head-1').click().waitForElementByCss('#head-1')
expect(await browser.elementByCss('meta[name="description"]').getAttribute('content')).toBe('Head One')
} finally {
if (browser) {
browser.close()
}
}
})
})
})
}
4 changes: 0 additions & 4 deletions yarn.lock
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -6455,10 +6455,6 @@ [email protected]:
version "4.0.0"
resolved "https://registry.yarnpkg.com/react-error-overlay/-/react-error-overlay-4.0.0.tgz#d198408a85b4070937a98667f500c832f86bd5d4"

[email protected]:
version "3.0.4"
resolved "https://registry.yarnpkg.com/react-lifecycles-compat/-/react-lifecycles-compat-3.0.4.tgz#4f1a273afdfc8f3488a8c516bfda78f872352362"

[email protected]:
version "5.4.0"
resolved "https://registry.yarnpkg.com/react-loadable/-/react-loadable-5.4.0.tgz#3b6b7d51121a7868fd155be848a36e02084742c9"
Expand Down