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.

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

Update Next.js to 9.4.4 #71

Merged
merged 79 commits into from
Jul 21, 2020
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
79 commits
Select commit Hold shift + click to select a range
db791d4
Update Next.js to 9.4.4
kmjennison Jul 15, 2020
31a358f
Delete .env file
kmjennison Jul 15, 2020
b794e02
Add 404 page
kmjennison Jul 15, 2020
5b95a56
Revert "Delete .env file"
kmjennison Jul 15, 2020
2d1847d
Move now.json to vercel.json
kmjennison Jul 15, 2020
54a5c8a
Try Next 9.4.2
kmjennison Jul 15, 2020
a57314f
Revert to Next 9.4.4
kmjennison Jul 15, 2020
fcdca79
Add TODO
kmjennison Jul 15, 2020
2f2de64
Use next/app
kmjennison Jul 16, 2020
04d7440
Add a test page
kmjennison Jul 16, 2020
8267f3a
Rename test page
kmjennison Jul 16, 2020
587214a
Try putting pages directly into pages dir
kmjennison Jul 16, 2020
907bf66
Try Next.js canary
kmjennison Jul 16, 2020
a58d3d4
Use built-in basePath and rewrite/redirect
kmjennison Jul 16, 2020
258a8af
Fix test
kmjennison Jul 16, 2020
46083e7
Add base path to server-side redirects
kmjennison Jul 16, 2020
733c162
Use trailing slash on Vercel
kmjennison Jul 16, 2020
cb0a3e8
Handle trailing slash in Next config
kmjennison Jul 16, 2020
5bc4eac
Add trailing slash in Vercel
kmjennison Jul 16, 2020
488d4a2
Try disabling Next trailing slash
kmjennison Jul 16, 2020
a861ee7
Support trailing slash rewrite in Next
kmjennison Jul 16, 2020
f10defd
Remove redirect
kmjennison Jul 16, 2020
20594e3
Try Vercel rewrites
kmjennison Jul 16, 2020
252b4be
Revert "Try Vercel rewrites"
kmjennison Jul 16, 2020
a0552d3
Try disabling trailing slash
kmjennison Jul 16, 2020
3dd1238
Try adding trailing slash in Vercel only
kmjennison Jul 16, 2020
6b0efb6
Disable trailing slash
kmjennison Jul 16, 2020
ee9b708
Update comments
kmjennison Jul 16, 2020
b6817c5
Disable trailing slash usage
kmjennison Jul 16, 2020
6e702dc
Add redirect
kmjennison Jul 16, 2020
a64a449
Revert "Try putting pages directly into pages dir"
kmjennison Jul 16, 2020
0696d45
Remove test page
kmjennison Jul 16, 2020
5d79186
Change scope of service worker
kmjennison Jul 16, 2020
fc791e8
Remove trailing slash from service worker scope
kmjennison Jul 16, 2020
54143c4
Try broadening service worker scope
kmjennison Jul 17, 2020
13886e4
Revert SW scope settings
kmjennison Jul 17, 2020
42c8f72
Try enforcing trailing slash on Vercel and rewriting to non-trailing …
kmjennison Jul 17, 2020
46429b0
Fix JSON
kmjennison Jul 17, 2020
c23f651
Revert test of Vercel rewrite
kmjennison Jul 17, 2020
0410f9e
Try undefined trailingSlash in vercel.json
kmjennison Jul 17, 2020
45c37bf
Add .vercel to gitignore
kmjennison Jul 17, 2020
68c463e
Comment out fake secret key to ensure we load the right one
kmjennison Jul 17, 2020
aeee720
Try requiring trailing slash in both Vercel and Next.js
kmjennison Jul 20, 2020
d0c01e5
Debug
kmjennison Jul 20, 2020
ad25e30
Revert "Debug"
kmjennison Jul 20, 2020
31243b8
Try latest Next build
kmjennison Jul 20, 2020
6f07cb0
Try disabling the base path
kmjennison Jul 20, 2020
2dec681
Use latest Next canary build
kmjennison Jul 20, 2020
233b24e
Add TODO
kmjennison Jul 20, 2020
7555e3b
Try removing Vercel trailing slash setting
kmjennison Jul 20, 2020
f7d7f15
Disable trailing slash
kmjennison Jul 20, 2020
4371653
Reenable trailing slash (404s on production; prev commit works)
kmjennison Jul 20, 2020
e8e13ad
Set undefined basePath setting
kmjennison Jul 20, 2020
4ed7c88
Try removing custom error page
kmjennison Jul 20, 2020
a40fc8c
Delete test for removed error page
kmjennison Jul 20, 2020
c9d2554
Revert "Delete test for removed error page"
kmjennison Jul 20, 2020
fc3b08d
Revert "Try removing custom error page"
kmjennison Jul 20, 2020
16f0d0c
Try disabling exportTrailingSlash
kmjennison Jul 20, 2020
34f6dbf
Try manually redirecting trailing slash in Vercel to non-trailing in …
kmjennison Jul 20, 2020
548ca19
Revert "Try manually redirecting trailing slash in Vercel to non-trai…
kmjennison Jul 20, 2020
f138f18
Set exportTrailingSlash to true
kmjennison Jul 20, 2020
8fe14f9
Update to latest Next canary
kmjennison Jul 20, 2020
117396b
Revert "Remove test page"
kmjennison Jul 20, 2020
ba51121
Try removing Next redirect
kmjennison Jul 20, 2020
94d6d22
Try disabling cleanUrls
kmjennison Jul 20, 2020
3d945f4
Disable trailing slash
kmjennison Jul 20, 2020
9c774f0
Revert "Disable trailing slash"
kmjennison Jul 20, 2020
cf183d1
Revert "Try disabling cleanUrls"
kmjennison Jul 20, 2020
fee0d6a
Make Vercel trailingSlash undefined
kmjennison Jul 20, 2020
afe7470
Try reverting to Next 9.4.4
kmjennison Jul 20, 2020
73ec3dc
Use Vercel to rewrite trailing slash and v4 base path
kmjennison Jul 20, 2020
5cf3c02
Try enabling experimental basePath with Vercel handling rewrites
kmjennison Jul 20, 2020
f083fed
Add comments
kmjennison Jul 21, 2020
740a49c
Use env var for trailing slash
kmjennison Jul 21, 2020
60f45f4
Use Vercel server in local development
kmjennison Jul 21, 2020
cabb3ad
Fix SW script URL
kmjennison Jul 21, 2020
ff355cf
Fix routing for SW script to include base path
kmjennison Jul 21, 2020
0f4e52e
Add newline
kmjennison Jul 21, 2020
09ad3be
Remove example
kmjennison Jul 21, 2020
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
15 changes: 10 additions & 5 deletions .env
Original file line number Diff line number Diff line change
@@ -1,4 +1,6 @@

# TODO: use Next's new env logic

# We must also define these variables in other places:
# * Add build time and runtime environment variable values to the
# ZEIT dashboard. As of 4/14/2020, this is is a beta feature:
Expand Down Expand Up @@ -30,7 +32,7 @@ FEATURE_FLAG_MOCK_ACHIEVEMENTS=true
FIREBASE_AUTH_DOMAIN=dev-tab-for-a-cause.firebaseapp.com
FIREBASE_CLIENT_EMAIL=next-js-firebase-admin-sdk@dev-tab-for-a-cause.iam.gserviceaccount.com
FIREBASE_DATABASE_URL=https://dev-tab-for-a-cause.firebaseio.com
FIREBASE_PRIVATE_KEY=some-key-here # secret
# FIREBASE_PRIVATE_KEY=some-key-here # secret
FIREBASE_PROJECT_ID=dev-tab-for-a-cause
FIREBASE_PUBLIC_API_KEY=AIzaSyDrGghKLnfOwwaSnPM0unRDXz_4YdorKU4

Expand All @@ -54,7 +56,10 @@ SESSION_COOKIE_SECURE_SAME_SITE_NONE=false
# SESSION_SECRET_CURRENT=some-secret
# SESSION_SECRET_PREVIOUS=a-previous-secret

# @area/workaround/next-js-base-path
URLS_BASE_PATH=""
URLS_USE_TRAILING_SLASH=false
URLS_API_BASE_PATH=""
# Note that the base path is also hardcoded in vercel.json and
# next.config.js.
URLS_BASE_PATH=/newtab
URLS_USE_TRAILING_SLASH=true
# To allow calls to this API from the legacy app
URLS_API_BASE_PATH=/v4

2 changes: 2 additions & 0 deletions .gitignore
Original file line number Diff line number Diff line change
Expand Up @@ -37,3 +37,5 @@ sessions
# https://github.com/whitecolor/yalc#keep-it-out-of-git
.yalc
yalc.lock

.vercel
43 changes: 42 additions & 1 deletion next.config.js
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,8 @@ const withSourceMaps = require('@zeit/next-source-maps')({
devtool: 'hidden-source-map'
})

const basePath = process.env.URLS_BASE_PATH

// Use the SentryWebpack plugin to upload the source maps during build.
const SentryWebpackPlugin = require('@sentry/webpack-plugin')

Expand All @@ -24,7 +26,46 @@ if (process.env.USE_LOCAL_ENV_FILE === 'true') {
}

const nextConfig = {
// For routing, we need:
// * the app to live on the /newtab base path.
// * to enforce a trailing slash on pages. This ensures a functional
// service worker and matches the URL in the Tab browser extension.
// * the /v4 path to rewrite to the /newtab path. This allows us to
// access the API from a different base path, which is important
// for routing through CloudFront with the legacy app.
// Next.js should soon have all the features we need for routing, but
// they're not yet stable. In v9.4.5-canary.41, setting trailingSlash
// to true in both Next.js and vercel.json apparently causes 404 errors.
// For now, we handle base path management here Next.js and enforce
// other route rewrites in vercel.json, stripping the trailing slash
// to route to Next.js.
experimental: {
// Should be stable in v9.4.5.
basePath: basePath,
},
exportTrailingSlash: true,

// We set the trailing slash preference in vercel.json.
// The trailing slash option is stable in v9.4.5-canary.41:
// https://github.com/vercel/next.js/releases/tag/v9.4.5-canary.41
// trailingSlash: true,

// Redirects should be available in v9.4.5.
// async redirects() {
// return [
// // Redirect from the index page to the base path index.
// // This is for convenience in local development and when
// // viewing preview deployments. It shouldn't need to be used
// // in production.
// (basePath && {
// source: `/`,
// destination: `${basePath}/`,
// basePath: false,
// permanent: false,
// }),
// ].filter(Boolean)
// },

// Public, build-time env vars.
// https://nextjs.org/docs#build-time-configuration
env: {
Expand All @@ -41,7 +82,7 @@ const nextConfig = {
SENTRY_DSN: process.env.SENTRY_DSN,
SERVICE_WORKER_ENABLED: process.env.SERVICE_WORKER_ENABLED,
URLS_API_BASE_PATH: process.env.URLS_API_BASE_PATH,
URLS_BASE_PATH: process.env.URLS_BASE_PATH, // @area/workaround/next-js-base-path
URLS_BASE_PATH: basePath,
URLS_USE_TRAILING_SLASH: process.env.URLS_USE_TRAILING_SLASH,
},
webpack: (config, options) => {
Expand Down
8 changes: 5 additions & 3 deletions package.json
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,8 @@
"version": "0.1.0",
"private": true,
"scripts": {
"dev": "USE_LOCAL_ENV_FILE=true NODE_ENV=development next dev",
"go": "USE_LOCAL_ENV_FILE=true NODE_ENV=development vercel dev",
"dev": "echo \"Use the 'yarn go' command to develop locally.\"",
"build": "NODE_ENV=test yarn run test && NODE_ENV=production yarn run relay && next build",
"start": "NODE_ENV=production next start",
"test": "yarn run lint && yarn run relay && jest",
Expand Down Expand Up @@ -31,7 +32,7 @@
"isomorphic-unfetch": "^3.0.0",
"lodash": "^4.17.15",
"moment": "^2.26.0",
"next": "^9.3.6",
"next": "^9.4.4",
"next-images": "^1.4.0",
"next-offline": "^5.0.2",
"prop-types": "^15.7.2",
Expand Down Expand Up @@ -70,6 +71,7 @@
"jest": "^26.0.1",
"mockdate": "^2.0.5",
"prettier": "^2.0.5",
"relay-compiler": "^9.1.0"
"relay-compiler": "^9.1.0",
"vercel": "^19.2.0"
}
}
12 changes: 1 addition & 11 deletions src/components/Link.js
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,6 @@ import PropTypes from 'prop-types'
import clsx from 'clsx'
import NextJsLink from 'next/link'
import { makeStyles } from '@material-ui/core/styles'
import { withBasePath } from 'src/utils/urls'

const useStyles = makeStyles(() => ({
anchor: {
Expand All @@ -17,17 +16,8 @@ const Link = (props) => {
const { children, className, to } = props
const classes = useStyles()

// We're disabling prefetch because it's broken by using
// an app subpath: see now.json "rewrites" and comments in
// urls.js. We can reenable prefetching after Next.js supports
// a "basePath" option.
// https://github.com/zeit/next.js/issues/4998#issuecomment-464345554
// We set the "as" parameter to fix client-side routing. This is a
// workaround for the missing "basePath" functionality:
// https://github.com/zeit/next.js/issues/4998#issuecomment-520888814
// @area/workaround/next-js-base-path
return (
<NextJsLink href={to} as={withBasePath(to)} prefetch={false}>
<NextJsLink href={to}>
<a className={clsx(classes.anchor, className)}>{children}</a>
</NextJsLink>
)
Expand Down
7 changes: 5 additions & 2 deletions src/components/Logo.js
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,7 @@ import tabLogoDefault from 'src/assets/logos/logo.svg'
import tabLogoWhite from 'src/assets/logos/logo-white.svg'
import tabLogoGrey from 'src/assets/logos/logo-grey.svg'
import tabLogoWithText from 'src/assets/logos/logo-with-text.svg'
import { withBasePath } from 'src/utils/urls'

const useStyles = makeStyles(() => ({
logoDefaults: {
Expand Down Expand Up @@ -44,13 +45,15 @@ const Logo = (props) => {
}
}
}
// The alt text flashes on Firefox
// The alt text flashes on Firefox.
// Note we need to manually add the basePath to assets:
// https://github.com/vercel/next.js/issues/4998#issuecomment-657233398
return (
// eslint-disable-next-line jsx-a11y/alt-text
<img
style={style}
className={clsx(classes.logo, className)}
src={logo}
src={withBasePath(logo)}
{...otherProps}
/>
)
Expand Down
25 changes: 0 additions & 25 deletions src/components/__tests__/Link.test.js
Original file line number Diff line number Diff line change
@@ -1,7 +1,6 @@
import React from 'react'
import { shallow } from 'enzyme'
import NextJsLink from 'next/link'
import { withBasePath } from 'src/utils/urls'

jest.mock('next/link', () => (props) => (
<div data-test-id="mock-next-js-link">{props.children}</div>
Expand All @@ -14,10 +13,6 @@ const getMockProps = () => ({
to: '/some/url',
})

beforeEach(() => {
withBasePath.mockImplementation((url) => `/my-fake-basepath${url}`)
})

describe('Link component', () => {
it('renders without error', () => {
const Link = require('src/components/Link').default
Expand Down Expand Up @@ -51,26 +46,6 @@ describe('Link component', () => {
expect(wrapper.at(0).prop('href')).toEqual('/my/thing')
})

it('sets Next.js Link "as" prop to use the basePath', () => {
const Link = require('src/components/Link').default
const mockProps = {
...getMockProps(),
to: '/my/thing',
}
const wrapper = shallow(<Link {...mockProps} />)
expect(wrapper.at(0).prop('href')).toEqual('/my/thing')
})

it('disables prefetch for the Next.js Link due to our basePath workaround', () => {
const Link = require('src/components/Link').default
const mockProps = {
...getMockProps(),
to: '/my/thing',
}
const wrapper = shallow(<Link {...mockProps} />)
expect(wrapper.at(0).prop('as')).toEqual('/my-fake-basepath/my/thing')
})

it('assigns the className prop to the anchor', () => {
const Link = require('src/components/Link').default
const mockProps = {
Expand Down
25 changes: 18 additions & 7 deletions src/components/__tests__/Logo.test.js
Original file line number Diff line number Diff line change
@@ -1,12 +1,19 @@
import React from 'react'
import { shallow } from 'enzyme'
import { withBasePath } from 'src/utils/urls'

jest.mock('src/utils/urls')

const getMockProps = () => ({
className: undefined,
color: 'purple',
includeText: false,
})

beforeAll(() => {
withBasePath.mockImplementation((val) => `/newtab/${val}`)
})

describe('Logo component', () => {
it('renders without error', () => {
const Logo = require('src/components/Logo').default
Expand Down Expand Up @@ -59,7 +66,7 @@ describe('Logo component', () => {
includeText: false,
}
const wrapper = shallow(<Logo {...mockProps} />)
expect(wrapper.find('img').prop('src')).toEqual('logo.svg')
expect(wrapper.find('img').prop('src')).toEqual('/newtab/logo.svg')
})

it('uses the correct file for color=default', () => {
Expand All @@ -69,7 +76,7 @@ describe('Logo component', () => {
color: 'default',
}
const wrapper = shallow(<Logo {...mockProps} />)
expect(wrapper.find('img').prop('src')).toEqual('logo.svg')
expect(wrapper.find('img').prop('src')).toEqual('/newtab/logo.svg')
})

it('uses the correct file for color=purple', () => {
Expand All @@ -79,7 +86,7 @@ describe('Logo component', () => {
color: 'purple',
}
const wrapper = shallow(<Logo {...mockProps} />)
expect(wrapper.find('img').prop('src')).toEqual('logo.svg')
expect(wrapper.find('img').prop('src')).toEqual('/newtab/logo.svg')
})

it('uses the correct file for color=white', () => {
Expand All @@ -89,7 +96,7 @@ describe('Logo component', () => {
color: 'white',
}
const wrapper = shallow(<Logo {...mockProps} />)
expect(wrapper.find('img').prop('src')).toEqual('logo-white.svg')
expect(wrapper.find('img').prop('src')).toEqual('/newtab/logo-white.svg')
})

it('uses the correct file for color=grey', () => {
Expand All @@ -99,7 +106,7 @@ describe('Logo component', () => {
color: 'grey',
}
const wrapper = shallow(<Logo {...mockProps} />)
expect(wrapper.find('img').prop('src')).toEqual('logo-grey.svg')
expect(wrapper.find('img').prop('src')).toEqual('/newtab/logo-grey.svg')
})

it('uses the correct file for includeText=true', () => {
Expand All @@ -109,7 +116,9 @@ describe('Logo component', () => {
includeText: true,
}
const wrapper = shallow(<Logo {...mockProps} />)
expect(wrapper.find('img').prop('src')).toEqual('logo-with-text.svg')
expect(wrapper.find('img').prop('src')).toEqual(
'/newtab/logo-with-text.svg'
)
})

it('uses the same file for includeText=true even when another (unsupported) color is provided', () => {
Expand All @@ -118,7 +127,9 @@ describe('Logo component', () => {
mockProps.includeText = true
mockProps.color = 'white'
const wrapper = shallow(<Logo {...mockProps} />)
expect(wrapper.find('img').prop('src')).toEqual('logo-with-text.svg')
expect(wrapper.find('img').prop('src')).toEqual(
'/newtab/logo-with-text.svg'
)
})

it('throws an error when passed an invalid color', () => {
Expand Down
20 changes: 9 additions & 11 deletions src/containers/_app.js
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@
/* eslint react/jsx-props-no-spreading: 0 */
import React, { useEffect } from 'react'
import PropTypes from 'prop-types'
import App from 'next/app'
import Head from 'next/head'
import { get, set } from 'lodash/object'
import * as Sentry from '@sentry/node'
Expand Down Expand Up @@ -35,7 +36,7 @@ if (process.env.SENTRY_DSN) {
console.warn(`SENTRY_DSN env var not defined. Not initializing Sentry.`)
}

const App = (props) => {
const MyApp = (props) => {
const { AuthUserInfo, Component, pageProps, err } = props

// Optionally, enable or disable the service worker:
Expand Down Expand Up @@ -126,7 +127,7 @@ const App = (props) => {
)
}

App.getInitialProps = async ({ Component, ctx }) => {
MyApp.getInitialProps = async ({ ctx, ...otherAppContext }) => {
const { req, res } = ctx

// Get the AuthUserInfo object.
Expand Down Expand Up @@ -164,18 +165,15 @@ App.getInitialProps = async ({ Component, ctx }) => {
AuthUserInfo
)

let pageProps = {}
if (Component.getInitialProps) {
pageProps = await Component.getInitialProps(ctx)
}
const appProps = await App.getInitialProps({ ctx, ...otherAppContext })
return {
pageProps,
...appProps,
AuthUserInfo,
}
}

App.displayName = 'App'
App.propTypes = {
MyApp.displayName = 'App'
MyApp.propTypes = {
AuthUserInfo: PropTypes.shape({
AuthUser: PropTypes.shape({
id: PropTypes.string.isRequired,
Expand All @@ -192,9 +190,9 @@ App.propTypes = {
pageProps: PropTypes.object,
}

App.defaultProps = {
MyApp.defaultProps = {
err: undefined,
pageProps: {},
}

export default App
export default MyApp
6 changes: 6 additions & 0 deletions src/pages/404.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,6 @@
import React from 'react'
import Error from 'next/error'

export default function NotFound() {
return <Error statusCode={404} />
}
Loading