Skip to content

Commit

Permalink
add eslint rule and adjustments
Browse files Browse the repository at this point in the history
  • Loading branch information
Alec Maliwanag authored and Alec Maliwanag committed Mar 23, 2021
1 parent 31c4d26 commit 0ec0639
Show file tree
Hide file tree
Showing 34 changed files with 101 additions and 19 deletions.
16 changes: 16 additions & 0 deletions .eslintrc.js
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,7 @@ module.exports = {
'react/no-unescaped-entities': 0,
'react/jsx-filename-extension': 0,
'react/jsx-one-expression-per-line': 0,

// The jsx-wrap-multilines rule conflicts with Prettier.
// https://github.com/prettier/prettier/issues/1009#issuecomment-286993938
'react/jsx-wrap-multilines': [
Expand All @@ -17,6 +18,21 @@ module.exports = {
},
],
'react-hooks/rules-of-hooks': 'error',
'lines-around-comment': [
'error',
{
beforeLineComment: true,
allowBlockStart: true,
allowBlockEnd: true,
allowObjectStart: true,
allowObjectEnd: true,
allowArrayStart: true,
allowArrayEnd: true,
allowClassStart: true,
allowClassEnd: true,
applyDefaultIgnorePatterns: true,
},
],
'react-hooks/exhaustive-deps': 'error',
'no-console': 'error',
},
Expand Down
13 changes: 8 additions & 5 deletions __mocks__/firebase/app.js
Original file line number Diff line number Diff line change
Expand Up @@ -4,22 +4,25 @@ const authMock = {
get currentUser() {
return null
},

// Takes a callback.
onAuthStateChanged: jest.fn(),

// Should resolve into a non-null Firebase user credential.
// https://firebase.google.com/docs/reference/js/firebase.auth.Auth?authuser=0#signInAnonymously
// `firebaseUser` should be non-null, so we should call
// `__setFirebaseUser` beforehand.
signInAnonymously: jest.fn(() =>
// Should resolve into a non-null Firebase user credential.
// https://firebase.google.com/docs/reference/js/firebase.auth.Auth?authuser=0#signInAnonymously
// `firebaseUser` should be non-null, so we should call
// `__setFirebaseUser` beforehand.
Promise.resolve({
credential: {},
user: null,
additionalUserInfo: {},
})
),
signOut: jest.fn(() => Promise.resolve()),

// Should resolve into a non-null Firebase user credential.
signInAndRetrieveDataWithCredential: jest.fn(() =>
// Should resolve into a non-null Firebase user credential.
Promise.resolve({
credential: {},
user: null,
Expand Down
1 change: 1 addition & 0 deletions __mocks__/next/router.js
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,7 @@ mock.useRouter = jest.fn(() => ({
push: jest.fn(),
reload: jest.fn(),
replace: jest.fn(),

// ... other things here
}))
module.exports = mock
1 change: 1 addition & 0 deletions babel.config.js
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,7 @@ module.exports = {
artifactDirectory: './src/relay/__generated__',
},
],

// Even though Next.js has built-in absolute imports, we need
// this for imports in Jest tests.
[
Expand Down
1 change: 1 addition & 0 deletions src/__tests__/pages/index.test.js
Original file line number Diff line number Diff line change
Expand Up @@ -445,6 +445,7 @@ describe('index.js', () => {
expect(LogUserRevenueMutation.mock.calls[0][0]).toEqual({
userId: 'asdf',
revenue: 0.0123,

// no encodedRevenue value
dfpAdvertiserId: '1111',
adSize: '728x90',
Expand Down
7 changes: 7 additions & 0 deletions src/components/Achievement.js
Original file line number Diff line number Diff line change
Expand Up @@ -448,22 +448,28 @@ Achievement.propTypes = {
// May want to accept descriptionMarkdown in the future.
description: PropTypes.string,
descriptionTwo: PropTypes.string,

// Whether the achievement has been completed or not.
status: PropTypes.oneOf([IN_PROGRESS, SUCCESS, FAILURE]).isRequired,

// ISO timestamp or null. Null if not yet completed.
// If successful, the successful completion time.
// If failed, the time the goal ended.
completionTime: PropTypes.string,

// ISO timestamp or null. Null if there is no deadline.
deadlineTime: PropTypes.string,

// Whether this is a goal for the entire Tab community. Default
// to false.
isCommunityGoal: PropTypes.bool,
progress: PropTypes.shape({
currentNumber: PropTypes.number.isRequired,
targetNumber: PropTypes.number.isRequired,

// How to visually display the progress.
visualizationType: PropTypes.oneOf([PROGRESS_BAR, CHECKMARKS]).isRequired,

// Perhaps add: leftLabelText, rightLabelText?
}),
shareButton: PropTypes.shape({
Expand All @@ -478,6 +484,7 @@ Achievement.propTypes = {
show: PropTypes.bool.isRequired,
text: PropTypes.string,
}),

// Content to show when sharing. Refer to the current Tab campaign
// logic. We may want to fetch this only after a user clicks
// to share.
Expand Down
4 changes: 4 additions & 0 deletions src/components/FirebaseAuth.js
Original file line number Diff line number Diff line change
Expand Up @@ -27,6 +27,7 @@ const FirebaseAuth = (props) => {
const firebaseAuthConfig = {
// Either 'popup' or 'redirect'
signInFlow: 'popup',

// Auth providers
// https://github.com/firebase/firebaseui-web#configure-oauth-providers
signInOptions: [
Expand Down Expand Up @@ -59,12 +60,15 @@ const FirebaseAuth = (props) => {
return false
},
},

// Just using the constant rather than importing firebaseui
// https://github.com/firebase/firebaseui-web#credential-helper
// https://github.com/firebase/firebaseui-web/blob/bd710448caa34c4a47a2fd578d76be8506d392d8/javascript/widgets/config.js#L83
credentialHelper: 'none',

// Terms of service URL
tosUrl: 'https://tab.gladly.io/terms/',

// Privacy policy URL
privacyPolicyUrl: 'https://tab.gladly.io/privacy/',
}
Expand Down
1 change: 1 addition & 0 deletions src/components/Logo.js
Original file line number Diff line number Diff line change
Expand Up @@ -54,6 +54,7 @@ const Logo = (props) => {
}
}
}

// 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
Expand Down
1 change: 1 addition & 0 deletions src/components/RandomGif.js
Original file line number Diff line number Diff line change
@@ -1,4 +1,5 @@
import React, { useState } from 'react'

// import PropTypes from 'prop-types'
import tickle from 'src/assets/gifs/tickle.gif'
import turntable from 'src/assets/gifs/turntable.gif'
Expand Down
2 changes: 2 additions & 0 deletions src/components/UserImpact.js
Original file line number Diff line number Diff line change
Expand Up @@ -62,6 +62,8 @@ const UserImpact = ({ userImpact, user }) => {
className={classes.impactCounter}
number={userImpactMetric}
progress={
// eslint-disable-next-line prettier/prettier

// if user achieves a new milestone show the progress bar as full
visitsUntilNextImpact === CAT_IMPACT_VISITS
? 100
Expand Down
1 change: 1 addition & 0 deletions src/components/__tests__/Achievement.test.js
Original file line number Diff line number Diff line change
Expand Up @@ -51,6 +51,7 @@ const getMockProps = () => ({
},
nextGoalButton: {
show: false,

// text: 'Next Goal' // it provides default text
},
description: 'Some info here.',
Expand Down
1 change: 1 addition & 0 deletions src/components/__tests__/NewTabThemeWrapperHOC.test.js
Original file line number Diff line number Diff line change
Expand Up @@ -32,6 +32,7 @@ describe('Theme Wrapper HOC', () => {
const WrappedHOC = flowRight([NewTabThemeWrapper])(DummyComponent)
const testComponent = shallow(<WrappedHOC />)
const themePassedIn = testComponent.props().theme

// make sure that our nested theme doesn't override the default theme on this value when enable background is disabled
expect(themePassedIn.palette.text.backgroundContrastText).toBeUndefined()
})
Expand Down
1 change: 1 addition & 0 deletions src/components/__tests__/UserImpact.test.js
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
import React from 'react'
import { shallow, mount } from 'enzyme'
import Button from '@material-ui/core/Button'

// import Typography from '@material-ui/core/Typography'
import Notification from 'src/components/Notification'
import ImpactDialog from 'src/components/ImpactDialog'
Expand Down
7 changes: 7 additions & 0 deletions src/pages/achievements.js
Original file line number Diff line number Diff line change
Expand Up @@ -107,6 +107,7 @@ const Achievements = () => {
taskText="Recruit 2 friends"
deadlineTime={dayjs().subtract(3, 'days').toISOString()}
completionTime={dayjs().subtract(3, 'days').toISOString()}

// progress={{
// currentNumber: 1,
// targetNumber: 2,
Expand All @@ -119,11 +120,14 @@ const Achievements = () => {
status="success"
taskText="Open tabs"
completionTime={dayjs().subtract(16, 'days').toISOString()}
// eslint-disable-next-line prettier/prettier

// progress={{
// currentNumber: 772,
// targetNumber: 1000,
// visualizationType: 'progressBar',
// }}
// eslint-disable-next-line react/jsx-props-no-multi-spaces
description="Tiramisu caramels jelly beans ice cream sesame snaps marshmallow lollipop pastry danish. Gummi bears oat cake donut cookie chocolate jelly jujubes. Muffin marzipan marshmallow danish oat cake. Chupa chups candy pastry."
descriptionTwo="Gummi bears oat cake donut cookie chocolate jelly jujubes."
isCommunityGoal
Expand All @@ -135,11 +139,14 @@ const Achievements = () => {
status="success"
taskText="Open 100 tabs"
completionTime={dayjs().subtract(21, 'days').toISOString()}
// eslint-disable-next-line prettier/prettier

// progress={{
// currentNumber: 100,
// targetNumber: 100,
// visualizationType: 'progressBar',
// }}
// eslint-disable-next-line react/jsx-props-no-multi-spaces
shareButton={{ show: true }}
/>
<Achievement
Expand Down
1 change: 1 addition & 0 deletions src/pages/beta-opt-in.js
Original file line number Diff line number Diff line change
@@ -1,4 +1,5 @@
import React from 'react'

// import PropTypes from 'prop-types'
import { unregister } from 'next-offline/runtime'
import Link from 'src/components/Link'
Expand Down
11 changes: 11 additions & 0 deletions src/pages/index.js
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,7 @@ import {
withAuthUserTokenSSR,
AuthAction,
} from 'next-firebase-auth'

// custom components
import Achievement from 'src/components/Achievement'
import Link from 'src/components/Link'
Expand All @@ -23,12 +24,14 @@ import UserBackgroundImageContainer from 'src/components/UserBackgroundImageCont
import UserImpactContainer from 'src/components/UserImpactContainer'
import SearchInput from 'src/components/SearchInput'
import NewTabThemeWrapperHOC from 'src/components/NewTabThemeWrapperHOC'

// material components
import { makeStyles } from '@material-ui/core/styles'
import grey from '@material-ui/core/colors/grey'
import Typography from '@material-ui/core/Typography'
import IconButton from '@material-ui/core/IconButton'
import SettingsIcon from '@material-ui/icons/Settings'

// utils
import withDataSSR from 'src/utils/pageWrappers/withDataSSR'
import withRelay from 'src/utils/pageWrappers/withRelay'
Expand Down Expand Up @@ -110,6 +113,7 @@ const useStyles = makeStyles((theme) => ({
transition: 'all 0.1s ease-in-out',
'&:hover': {
transform: 'scale(1.01)',

// TODO
// Increase elevation on hover.
},
Expand Down Expand Up @@ -201,6 +205,7 @@ if (isClientSide()) {
bidderTimeout: 700,
consent: {
enabled: true,

// Time to wait for the consent management platform (CMP) to respond.
// If the CMP does not respond in this time, ad auctions may be cancelled.
// The tab-cmp package aims to make the CMP respond much more quickly
Expand Down Expand Up @@ -262,6 +267,7 @@ const Index = ({ data: initialData }) => {
const { data } = useData({
getRelayQuery,
initialData,

// If we are using the service worker (serving a cached version
// of the page HTML), fetch fresh data on mount.
...(process.env.NEXT_PUBLIC_SERVICE_WORKER_ENABLED === 'true' && {
Expand All @@ -270,12 +276,14 @@ const Index = ({ data: initialData }) => {
})
const showAchievements = showMockAchievements()
const enableBackgroundImages = showBackgroundImages()

// Determine which ad units we'll show only once, on mount,
// because the ads have already been fetched and won't change.
const [adUnits, setAdUnits] = useState([])
useEffect(() => {
setAdUnits(getAdUnits())
}, [])

// Only render ads if we are on the client side.
const [shouldRenderAds, setShouldRenderAds] = useState(false)
useEffect(() => {
Expand All @@ -292,6 +300,7 @@ const Index = ({ data: initialData }) => {
// toggling state and a rerender when we successfully fire the
// SetHasViewedIntroFlowMutation
const [justFinishedIntroFlow, setJustFinishedIntroFlow] = useState(false)

// log tab count when user first visits
useEffect(() => {
if (userGlobalId && tabId) {
Expand All @@ -309,6 +318,7 @@ const Index = ({ data: initialData }) => {
if (!data) {
return <FullPageLoader />
}

// Data to provide the onAdDisplayed callback
const adContext = {
user,
Expand Down Expand Up @@ -352,6 +362,7 @@ const Index = ({ data: initialData }) => {
}),
dfpAdvertiserId: GAMAdvertiserId.toString(),
adSize,

// Only send aggregationOperation value if we have more than one
// revenue value
aggregationOperation:
Expand Down
10 changes: 5 additions & 5 deletions src/utils/adHelpers.js
Original file line number Diff line number Diff line change
Expand Up @@ -20,11 +20,10 @@ const DEFAULT_NUMBER_OF_ADS = 2
* today, using tab count as a proxy.
* @return {Boolean} Whether the user has viewed the max ads today.
*/
const hasUserReachedMaxTabsToday = () =>
// If the user has exceeded the daily tab maximum,
// do not show ads.
// https://github.com/gladly-team/tab/issues/202
false
// If the user has exceeded the daily tab maximum,
// do not show ads.
// https://github.com/gladly-team/tab/issues/202
const hasUserReachedMaxTabsToday = () => false

// TODO: implement
/**
Expand Down Expand Up @@ -68,6 +67,7 @@ export const getAdUnits = () => {
...(numberOfAdsToShow > 2 && { rectangleAdSecondary }),
}
}

/**
* Determine if we should fetch and display ads. Ads are disabled
* by env variable or if the user views a lot of ads in a single day.
Expand Down
3 changes: 3 additions & 0 deletions src/utils/auth/initAuth.js
Original file line number Diff line number Diff line change
Expand Up @@ -21,6 +21,7 @@ const useSecureSameSiteNone =
process.env.COOKIE_SECURE_SAME_SITE_NONE === 'true'
const tokenChangedHandler = async (authUser) => {
let response

// If the user is authed, call login to set a cookie.
if (authUser.id) {
const userToken = await authUser.getIdToken()
Expand Down Expand Up @@ -71,6 +72,7 @@ const initAuth = () => {
credential: {
projectId: process.env.NEXT_PUBLIC_FIREBASE_PROJECT_ID,
clientEmail: process.env.NEXT_PUBLIC_FIREBASE_CLIENT_EMAIL,

// Our approach:
// https://github.com/vercel/vercel/issues/749#issuecomment-707515089
// Another potential approach:
Expand All @@ -97,6 +99,7 @@ const initAuth = () => {
maxAge: 12 * 60 * 60 * 24 * 1000, // twelve days
overwrite: true,
path: '/',

// Important that production serves sameSite=None and secure=true
// because we may load this page as an iframe on the new tab page
// (cross-domain).
Expand Down
1 change: 1 addition & 0 deletions src/utils/caching.js
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@
/* globals window */
import { isClientSide } from 'src/utils/ssr'
import debounce from 'lodash/debounce'

// Delete all cached data.
export const clearAllServiceWorkerCaches = async () => {
if (!isClientSide()) {
Expand Down
1 change: 1 addition & 0 deletions src/utils/ensureValuesAreDefined.js
Original file line number Diff line number Diff line change
Expand Up @@ -26,6 +26,7 @@ const ensureValuesAreDefined = (value) => {
if (!value.length) {
hasNilVal = true
}

// Test each entry in the array.
value.forEach((item) => {
if (isNil(item) || item === '') {
Expand Down
Loading

0 comments on commit 0ec0639

Please sign in to comment.