Skip to content

Commit

Permalink
Replace url-parse with URL (#3671)
Browse files Browse the repository at this point in the history
* Replace url-parse with URL
Replace the `url-parse` package with the native URL interface, which is supported in Node and in browsers.

* Use URL objects for redirects

Co-authored-by: Mark Bouslog <mcbouslog@gmail.com>
  • Loading branch information
eatyourgreens and mcbouslog committed Sep 21, 2022
1 parent 6d9fde2 commit 169fed2
Show file tree
Hide file tree
Showing 12 changed files with 138 additions and 125 deletions.
1 change: 0 additions & 1 deletion packages/app-content-pages/package.json
Original file line number Diff line number Diff line change
Expand Up @@ -48,7 +48,6 @@
"snazzy": "^8.0.0",
"styled-components": "~5.3.3",
"superagent": "~8.0.0",
"url-parse": "~1.5.1",
"validator": "~13.7.0"
},
"devDependencies": {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -2,18 +2,17 @@ import { inject, observer } from 'mobx-react'
import { withRouter } from 'next/router'
import { func, shape, string } from 'prop-types'
import { Component } from 'react'
import Url from 'url-parse'

import AuthModal from './AuthModal'

class AuthModalContainer extends Component {
constructor () {
constructor() {
super()
this.closeModal = this.closeModal.bind(this)
this.onActive = this.onActive.bind(this)
}

onActive (activeIndex) {
onActive(activeIndex) {
const url = this.getUrlObject()

if (activeIndex === 0) {
Expand All @@ -27,53 +26,49 @@ class AuthModalContainer extends Component {
this.redirect(url)
}

closeModal () {
closeModal() {
const url = this.getUrlObject()
this.removeUrlQuery(url, 'login')
this.removeUrlQuery(url, 'register')
return this.redirect(url)
}

getActiveIndexFromUrl () {
const { query } = this.getUrlObject()
const params = {
login: query.login === 'true',
register: query.register === 'true'
}
getActiveIndexFromUrl() {
const url = this.getUrlObject()

let activeIndex = -1

if (params.login) {
if (url.searchParams?.has('login')) {
activeIndex = 0
} else if (params.register) {
} else if (url.searchParams?.has('register')) {
activeIndex = 1
}

return activeIndex
}

getUrlObject () {
const { asPath } = this.props.router
return new Url(asPath, true)
getUrlObject() {
const isBrowser = typeof window !== 'undefined'
if (isBrowser) {
return new URL(window.location)
}
return ''
}

redirect (urlObject) {
const { pathname, push } = this.props.router
const urlString = urlObject.toString().substr(urlObject.origin.length)
push(pathname, urlString, { shallow: true })
redirect(urlObject) {
const { push } = this.props.router
push(urlObject, urlObject, { shallow: true })
}

addUrlQuery (urlObject, paramToAdd) {
urlObject.set('query', { [paramToAdd]: true, ...urlObject.query })
addUrlQuery(urlObject, paramToAdd) {
urlObject.searchParams.set(paramToAdd, true)
}

removeUrlQuery (urlObject, paramToRemove) {
const query = { ...urlObject.query }
delete query[paramToRemove]
urlObject.set('query', query)
removeUrlQuery(urlObject, paramToRemove) {
urlObject.searchParams.delete(paramToRemove)
}

render () {
render() {
const activeIndex = this.getActiveIndexFromUrl()
return (
<AuthModal
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -29,17 +29,25 @@ describe('Component > AuthModalContainer', function () {
})

it('should pass a prop to show the Login tab if there is a matching url query', function () {
global.dom.reconfigure({
url: 'https://localhost/foo/bar?login=true'
})
const loginWrapper = shallow(<AuthModalContainer router={ROUTER} />)
expect(getChildProp(loginWrapper, 'activeIndex')).to.equal(-1)
loginWrapper.setProps({ router: { ...ROUTER, asPath: '/?login=true' } })
expect(getChildProp(loginWrapper, 'activeIndex')).to.equal(0)
global.dom.reconfigure({
url: 'https://localhost'
})
})

it('should pass a prop to show the Register tab if there is a matching url query', function () {
global.dom.reconfigure({
url: 'https://localhost/foo/bar?register=true'
})
const loginWrapper = shallow(<AuthModalContainer router={ROUTER} />)
expect(getChildProp(loginWrapper, 'activeIndex')).to.equal(-1)
loginWrapper.setProps({ router: { ...ROUTER, asPath: '/?register=true' } })
expect(getChildProp(loginWrapper, 'activeIndex')).to.equal(1)
global.dom.reconfigure({
url: 'https://localhost'
})
})
})

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -4,72 +4,93 @@ import { withRouter } from 'next/router'
import auth from 'panoptes-client/lib/auth'
import { bool, func, shape, string } from 'prop-types'
import { Component } from 'react'
import Url from 'url-parse'

function storeMapper (stores) {
function storeMapper(stores) {
const { user } = stores.store
return {
user
}
}

class ZooHeaderWrapperContainer extends Component {
constructor () {
constructor() {
super()
this.openRegisterModal = this.openRegisterModal.bind(this)
this.openSignInModal = this.openSignInModal.bind(this)
this.signOut = this.signOut.bind(this)
}

createUserProp () {
createUserProp() {
const { user } = this.props
return (user.isLoggedIn)
? { 'display_name': user['display_name'] }
? { display_name: user.display_name, login: user.login }
: {}
}

getUrlObject () {
const { asPath } = this.props.router
return new Url(asPath, true)
getUrlObject() {
return new URL(window.location)
}

addUrlQuery (urlObject, propertyToAdd) {
const query = Object.assign({}, urlObject.query, {
[propertyToAdd]: 'true'
})
urlObject.set('query', query)
addUrlQuery(urlObject, propertyToAdd) {
urlObject.searchParams.set(propertyToAdd, true)
}

redirect (urlObject) {
const { pathname, push } = this.props.router
const urlString = urlObject.toString().substr(urlObject.origin.length)
push(pathname, urlString, { shallow: true })
redirect(urlObject) {
const { push } = this.props.router
push(urlObject, urlObject, { shallow: true })
}

openRegisterModal () {
openRegisterModal() {
const url = this.getUrlObject()
this.addUrlQuery(url, 'register')
return this.redirect(url)
}

openSignInModal () {
openSignInModal() {
const url = this.getUrlObject()
this.addUrlQuery(url, 'login')
return this.redirect(url)
}

signOut () {
auth.signOut()
.then(() => this.props.user.clear())
signOut() {
return auth.signOut()
.then(() => {
this.props.user.clear()
// resetting the alreay seen subjects during the session tracking should move
// once we refactor the UPP and User resource tracking in the classifier
// Current implementation in classifier is possibly buggy (see discussion https://github.com/zooniverse/front-end-monorepo/discussions/2362)
// likely a user id prop will be passed to the classifier and that will be reacted to with an effect hook
/// which would reset the subject already seen tracking
// I needed to guarantee that this happened on sign out only so that's why this is here for now
const seenThisSession = (window) ? window.sessionStorage.getItem("subjectsSeenThisSession") : null

if (seenThisSession) {
window.sessionStorage.removeItem("subjectsSeenThisSession")
}
})
}

unreadMessages() {
const { user } = this.props

return user?.personalization?.notifications?.unreadConversationsIds.length
}

render () {
unreadNotifications() {
const { user } = this.props

return user?.personalization?.notifications?.unreadNotificationsCount
}

render() {
return (
<ZooHeader
{...this.props}
register={this.openRegisterModal}
signIn={this.openSignInModal}
signOut={this.signOut}
unreadMessages={this.unreadMessages()}
unreadNotifications={this.unreadNotifications()}
user={this.createUserProp()}
/>
)
Expand All @@ -85,10 +106,12 @@ ZooHeaderWrapperContainer.propTypes = {
user: shape({
clear: func,
display_name: string,
login: string,
isLoggedIn: bool
})
}


@withRouter
@inject(storeMapper)
@observer
Expand Down
1 change: 1 addition & 0 deletions packages/app-content-pages/test/create-global-document.js
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,7 @@ function copyProps (src, target) {
Object.defineProperties(target, props)
}

global.dom = jsdom
global.window = window
global.document = window.document
global.navigator = {
Expand Down
1 change: 0 additions & 1 deletion packages/app-project/package.json
Original file line number Diff line number Diff line change
Expand Up @@ -63,7 +63,6 @@
"styled-components": "~5.3.3",
"svg-loaders-react": "~2.2.1",
"swr": "~1.3.0",
"url-parse": "~1.5.1",
"validator": "~13.7.0"
},
"devDependencies": {
Expand Down
47 changes: 21 additions & 26 deletions packages/app-project/src/components/AuthModal/AuthModalContainer.js
Original file line number Diff line number Diff line change
Expand Up @@ -2,18 +2,17 @@ import { inject, observer } from 'mobx-react'
import { withRouter } from 'next/router'
import { func, shape, string } from 'prop-types'
import { Component } from 'react'
import Url from 'url-parse'

import AuthModal from './AuthModal'

class AuthModalContainer extends Component {
constructor () {
constructor() {
super()
this.closeModal = this.closeModal.bind(this)
this.onActive = this.onActive.bind(this)
}

onActive (activeIndex) {
onActive(activeIndex) {
const url = this.getUrlObject()

if (activeIndex === 0) {
Expand All @@ -27,53 +26,49 @@ class AuthModalContainer extends Component {
this.redirect(url)
}

closeModal () {
closeModal() {
const url = this.getUrlObject()
this.removeUrlQuery(url, 'login')
this.removeUrlQuery(url, 'register')
return this.redirect(url)
}

getActiveIndexFromUrl () {
const { query } = this.getUrlObject()
const params = {
login: query.login === 'true',
register: query.register === 'true'
}
getActiveIndexFromUrl() {
const url = this.getUrlObject()

let activeIndex = -1

if (params.login) {
if (url.searchParams?.has('login')) {
activeIndex = 0
} else if (params.register) {
} else if (url.searchParams?.has('register')) {
activeIndex = 1
}

return activeIndex
}

getUrlObject () {
const { asPath } = this.props.router
return new Url(asPath, true)
getUrlObject() {
const isBrowser = typeof window !== 'undefined'
if (isBrowser) {
return new URL(window.location)
}
return ''
}

redirect (urlObject) {
const { pathname, push } = this.props.router
const urlString = urlObject.toString().substr(urlObject.origin.length)
push(pathname, urlString, { shallow: true })
redirect(urlObject) {
const { push } = this.props.router
push(urlObject, urlObject, { shallow: true })
}

addUrlQuery (urlObject, paramToAdd) {
urlObject.set('query', { [paramToAdd]: true, ...urlObject.query })
addUrlQuery(urlObject, paramToAdd) {
urlObject.searchParams.set(paramToAdd, true)
}

removeUrlQuery (urlObject, paramToRemove) {
const query = { ...urlObject.query }
delete query[paramToRemove]
urlObject.set('query', query)
removeUrlQuery(urlObject, paramToRemove) {
urlObject.searchParams.delete(paramToRemove)
}

render () {
render() {
const activeIndex = this.getActiveIndexFromUrl()
return (
<AuthModal
Expand Down
Loading

0 comments on commit 169fed2

Please sign in to comment.