Skip to content

Commit

Permalink
Replace getInitialProps with getServerSideProps (#11586)
Browse files Browse the repository at this point in the history
  • Loading branch information
@djanda97
djanda97 authored Apr 2, 2020
1 parent 97a6b64 commit 641976a
Showing 1 changed file with 13 additions and 8 deletions.
21 changes: 13 additions & 8 deletions examples/with-firebase-authentication/pages/index.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -5,16 +5,21 @@ import 'firebase/firestore'
import 'isomorphic-unfetch'
import clientCredentials from '../credentials/client'

export default class Index extends Component {
static async getInitialProps({ req, query }) {
const user = req && req.session ? req.session.decodedToken : null
// don't fetch anything from firebase if the user is not found
// const snap = user && await req.firebaseServer.database().ref('messages').once('value')
// const messages = snap && snap.val()
const messages = null
return { user, messages }
export async function getServerSideProps({ req, query }) {

This comment has been minimized.

Sign in to view

Copy link
@msheakoski

msheakoski Apr 3, 2020

Contributor

query does not seem to be used in this function and can be removed.
const user = req && req.session ? req.session.decodedToken : null

This comment has been minimized.

Sign in to view

Copy link
@msheakoski

msheakoski Apr 3, 2020

Contributor

With the change to getServerSideProps, req is always available, so the check can be simplified to:

const user = req.session ? req.session.decodedToken : null

This comment has been minimized.

Sign in to view

Copy link
@clarencejychan

clarencejychan Apr 6, 2020

Won't this actually fail on first load if there's no decodedToken?

Given the scenario where the user hasn't been logged in yet, you could still have a session started but you won't have a decodedToken since the /login route would have never been hit yet.

This comment has been minimized.

Sign in to view

Copy link
@msheakoski

msheakoski Apr 6, 2020

Contributor

The currently committed code also assumes that just because there is a session started, decodedToken must exist:

const user = req && req.session ? req.session.decodedToken : null

I only removed the check for req since that is guaranteed to be available in getServerSideProps. It looks like maybe the code should instead be:

const user = req.session && req.session.decodedToken ? req.session.decodedToken : null

or if optional chaining and nullish coalescing are allowed:

const user = req?.session?.decodedToken ?? null

This comment has been minimized.

Sign in to view

Copy link
@clarencejychan

clarencejychan Apr 6, 2020

First option looks better to me. As long as there's a check there seems to be okay! Mind if I make a quick PR for that or will you do so?

This comment has been minimized.

Sign in to view

Copy link
@msheakoski

msheakoski Apr 6, 2020

Contributor

@clarencejychan Absolutely! The PR is all yours :)
// don't fetch anything from firebase if the user is not found
// const snap = user && await req.firebaseServer.database().ref('messages').once('value')
// const messages = snap && snap.val()
const messages = null
return {
props: {
user,
messages,
},
}
}

export default class Index extends Component {
constructor(props) {
super(props)
this.state = {
Expand Down

0 comments on commit 641976a

Please sign in to comment.