Skip to content

Commit

Permalink
don't get available languages from session (github#29715)
Browse files Browse the repository at this point in the history
* don't get available languages from session

* update useSession

* one more fix
  • Loading branch information
peterbe authored Aug 8, 2022
1 parent f73fb93 commit a1e3866
Show file tree
Hide file tree
Showing 14 changed files with 67 additions and 93 deletions.
5 changes: 2 additions & 3 deletions components/Search.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -6,13 +6,13 @@ import { Flash, Label, ActionList, ActionMenu } from '@primer/react'
import { ItemInput } from '@primer/react/lib/deprecated/ActionList/List'
import { InfoIcon } from '@primer/octicons-react'

import { useLanguages } from 'components/context/LanguagesContext'
import { useTranslation } from 'components/hooks/useTranslation'
import { sendEvent, EventType } from 'components/lib/events'
import { useMainContext } from './context/MainContext'
import { DEFAULT_VERSION, useVersion } from 'components/hooks/useVersion'
import { useQuery } from 'components/hooks/useQuery'
import { Link } from 'components/Link'
import { useSession } from 'components/hooks/useSession'

import styles from './Search.module.scss'

Expand Down Expand Up @@ -56,8 +56,7 @@ export function Search({
const inputRef = useRef<HTMLInputElement>(null)
const { t } = useTranslation('search')
const { currentVersion } = useVersion()
const { session } = useSession()
const languages = session?.languages
const { languages } = useLanguages()

// Figure out language and version for index
const { searchVersions, nonEnterpriseDefaultVersion } = useMainContext()
Expand Down
24 changes: 24 additions & 0 deletions components/context/LanguagesContext.tsx
Original file line number Diff line number Diff line change
@@ -0,0 +1,24 @@
import { createContext, useContext } from 'react'

type LanguageItem = {
name: string
nativeName?: string
code: string
hreflang: string
}

export type LanguagesContextT = {
languages: Record<string, LanguageItem>
}

export const LanguagesContext = createContext<LanguagesContextT | null>(null)

export const useLanguages = (): LanguagesContextT => {
const context = useContext(LanguagesContext)

if (!context) {
throw new Error('"useLanguagesContext" may only be used inside "LanguagesContext.Provider"')
}

return context
}
9 changes: 0 additions & 9 deletions components/hooks/useSession.ts
Original file line number Diff line number Diff line change
Expand Up @@ -9,19 +9,10 @@ export default async function fetcher<JSON = any>(
return res.json()
}

type LanguageItem = {
name: string
nativeName?: string
code: string
hreflang: string
wip?: boolean
}

export type Session = {
isSignedIn: boolean
csrfToken: string
userLanguage: string // en, es, ja, cn
languages: Record<string, LanguageItem>
theme: {
colorMode: Pick<ThemeProviderProps, 'colorMode'>
nightTheme: string
Expand Down
12 changes: 4 additions & 8 deletions components/page-header/HeaderNotifications.tsx
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
import { useRouter } from 'next/router'
import cx from 'classnames'

import { useLanguages } from 'components/context/LanguagesContext'
import { useMainContext } from 'components/context/MainContext'
import { useTranslation } from 'components/hooks/useTranslation'
import { ExcludesNull } from 'components/lib/ExcludesNull'
Expand All @@ -24,13 +25,13 @@ export const HeaderNotifications = () => {
const { relativePath, allVersions, data, currentPathWithoutLanguage, page } = useMainContext()
const { session } = useSession()
const userLanguage = session?.userLanguage
const languages = session?.languages || {}
const { languages } = useLanguages()

const { t } = useTranslation('header')

const translationNotices: Array<Notif> = []
if (router.locale === 'en') {
if (userLanguage && userLanguage !== 'en' && languages[userLanguage]?.wip === false) {
if (userLanguage && userLanguage !== 'en') {
translationNotices.push({
type: NotificationType.TRANSLATION,
content: `This article is also available in <a href="/${userLanguage}${currentPathWithoutLanguage}">${languages[userLanguage]?.name}</a>.`,
Expand All @@ -42,16 +43,11 @@ export const HeaderNotifications = () => {
type: NotificationType.TRANSLATION,
content: data.reusables.policies.translation,
})
} else if (router.locale && languages[router.locale]?.wip !== true) {
} else if (router.locale) {
translationNotices.push({
type: NotificationType.TRANSLATION,
content: t('notices.localization_complete'),
})
} else if (router.locale && languages[router.locale]?.wip) {
translationNotices.push({
type: NotificationType.TRANSLATION,
content: t('notices.localization_in_progress'),
})
}
}
const releaseNotices: Array<Notif> = []
Expand Down
24 changes: 10 additions & 14 deletions components/page-header/LanguagePicker.tsx
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
import { useRouter } from 'next/router'
import Cookies from 'js-cookie'

import { useSession } from 'components/hooks/useSession'
import { useLanguages } from 'components/context/LanguagesContext'
import { Picker } from 'components/ui/Picker'
import { useTranslation } from 'components/hooks/useTranslation'

Expand All @@ -14,14 +14,12 @@ type Props = {

export const LanguagePicker = ({ variant }: Props) => {
const router = useRouter()
const { session } = useSession()
const languages = session?.languages
const { languages } = useLanguages()

const locale = router.locale || 'en'

const { t } = useTranslation('picker')

if (!languages) return null

const langs = Object.values(languages)
const selectedLang = languages[locale]

Expand Down Expand Up @@ -52,15 +50,13 @@ export const LanguagePicker = ({ variant }: Props) => {
<Picker
variant={variant}
defaultText={t('language_picker_default_text')}
options={langs
.filter((lang) => !lang.wip)
.map((lang) => ({
text: lang.nativeName || lang.name,
selected: lang === selectedLang,
locale: lang.code,
href: `${routerPath}`,
onselect: rememberPreferredLanguage,
}))}
options={langs.map((lang) => ({
text: lang.nativeName || lang.name,
selected: lang === selectedLang,
locale: lang.code,
href: `${routerPath}`,
onselect: rememberPreferredLanguage,
}))}
/>
</div>
)
Expand Down
5 changes: 0 additions & 5 deletions lib/languages.js
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,6 @@ const languages = {
code: 'en',
hreflang: 'en',
dir: '',
wip: false,
},
cn: {
name: 'Simplified Chinese',
Expand All @@ -15,7 +14,6 @@ const languages = {
hreflang: 'zh-Hans',
redirectPatterns: [/^\/zh-\w{2}/, /^\/zh/],
dir: 'translations/zh-CN',
wip: false,
},
ja: {
name: 'Japanese',
Expand All @@ -24,23 +22,20 @@ const languages = {
hreflang: 'ja',
redirectPatterns: [/^\/jp/],
dir: 'translations/ja-JP',
wip: false,
},
es: {
name: 'Spanish',
nativeName: 'Español',
code: 'es',
hreflang: 'es',
dir: 'translations/es-ES',
wip: false,
},
pt: {
name: 'Portuguese',
nativeName: 'Português do Brasil',
code: 'pt',
hreflang: 'pt',
dir: 'translations/pt-BR',
wip: false,
},
}

Expand Down
1 change: 0 additions & 1 deletion middleware/api/session.js
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,6 @@ router.get('/', (req, res) => {
isSignedIn: Boolean(req.cookies?.dotcom_user),
csrfToken: req.csrfToken?.() || '',
userLanguage: req.userLanguage,
languages: req.context.languages,
theme: getTheme(req),
themeCss: getTheme(req, true),
})
Expand Down
6 changes: 0 additions & 6 deletions middleware/block-robots.js
Original file line number Diff line number Diff line change
@@ -1,13 +1,7 @@
import languages from '../lib/languages.js'
import { productMap } from '../lib/all-products.js'
import { deprecated } from '../lib/enterprise-server-releases.js'

const pathRegExps = [
// Disallow indexing of WIP localized content
...Object.values(languages)
.filter((language) => language.wip)
.map((language) => new RegExp(`^/${language.code}(/.*)?$`, 'i')),

// Disallow indexing of WIP products
...Object.values(productMap)
.filter((product) => product.wip || product.hidden)
Expand Down
3 changes: 1 addition & 2 deletions middleware/detect-language.js
Original file line number Diff line number Diff line change
Expand Up @@ -36,8 +36,7 @@ function getUserLanguage(browserLanguages) {

function getUserLanguageFromCookie(req) {
const value = req.cookies[PREFERRED_LOCALE_COOKIE_NAME]
// But if it's a WIP language, reject it.
if (value && languages[value] && !languages[value].wip) {
if (value && languages[value]) {
return value
}
}
Expand Down
15 changes: 13 additions & 2 deletions pages/_app.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -8,15 +8,17 @@ import '../stylesheets/index.scss'

import events from 'components/lib/events'
import experiment from 'components/lib/experiment'
import { LanguagesContext, LanguagesContextT } from 'components/context/LanguagesContext'
import { useSession } from 'components/hooks/useSession'

type MyAppProps = AppProps & {
isDotComAuthenticated: boolean
languagesContext: LanguagesContextT
}

type colorModeAuto = Pick<ThemeProviderProps, 'colorMode'>

const MyApp = ({ Component, pageProps }: MyAppProps) => {
const MyApp = ({ Component, pageProps, languagesContext }: MyAppProps) => {
const { session, isLoadingSession } = useSession()
useEffect(() => {
events(session?.csrfToken)
Expand Down Expand Up @@ -67,7 +69,9 @@ const MyApp = ({ Component, pageProps }: MyAppProps) => {
{ opacity: isLoadingSession ? 0.1 : 1 }
}
>
<Component {...pageProps} />
<LanguagesContext.Provider value={languagesContext}>
<Component {...pageProps} />
</LanguagesContext.Provider>
</div>
</ThemeProvider>
</SSRProvider>
Expand All @@ -80,11 +84,18 @@ const MyApp = ({ Component, pageProps }: MyAppProps) => {
// executed every time **in the client** if it was the first time
// ever (since restart) or from a cached HTML.
MyApp.getInitialProps = async (appContext: AppContext) => {
const { ctx } = appContext
// calls page's `getInitialProps` and fills `appProps.pageProps`
const appProps = await App.getInitialProps(appContext)
const req: any = ctx.req

// Have to define the type manually here because `req.context.languages`
// comes from Node JS and is not type-aware.
const languages: LanguagesContextT = req.context.languages

return {
...appProps,
languagesContext: { languages },
}
}

Expand Down
2 changes: 1 addition & 1 deletion script/i18n/test-html-pages.js
Original file line number Diff line number Diff line change
Expand Up @@ -40,7 +40,7 @@ async function main() {
const languageCodes =
[languageCode] ||
Object.keys(languages)
.filter((language) => !language.wip && language !== 'en')
.filter((language) => language !== 'en')
.map((language) => languages[language].code)
const versions = singleVersion ? [singleVersion] : Object.keys(allVersions)

Expand Down
17 changes: 0 additions & 17 deletions tests/browser/browser.js
Original file line number Diff line number Diff line change
@@ -1,6 +1,5 @@
import { jest } from '@jest/globals'
import { latest, oldestSupported } from '../../lib/enterprise-server-releases.js'
import languages from '../../lib/languages.js'

jest.useFakeTimers({ legacyFakeTimers: true })

Expand Down Expand Up @@ -421,22 +420,6 @@ describe('filter cards', () => {
})
})

describe('language banner', () => {
it('directs user to the English version of the article', async () => {
const wipLanguageKey = Object.keys(languages).find((key) => languages[key].wip)

// This kinda sucks, but if we don't have a WIP language, we currently can't
// run a reliable test. But hey, on the bright side, if we don't have a WIP
// language then this code will never run anyway!
if (wipLanguageKey) {
const res = await page.goto(`http://localhost:4000/${wipLanguageKey}/actions`)
expect(res.ok()).toBe(true)
const href = await page.$eval('a#to-english-doc', (el) => el.href)
expect(href.endsWith('/en/actions')).toBe(true)
}
})
})

// Skipping because next/links are disabled by default for now
// Docs Engineering issue: 962
describe.skip('next/link client-side navigation', () => {
Expand Down
19 changes: 4 additions & 15 deletions tests/rendering/block-robots.js
Original file line number Diff line number Diff line change
Expand Up @@ -15,21 +15,10 @@ describe('block robots', () => {
})

it('allows crawling of generally available localized content', async () => {
Object.values(languages)
.filter((language) => !language.wip)
.forEach((language) => {
expect(allowIndex(`/${language.code}`)).toBe(true)
expect(allowIndex(`/${language.code}/articles/verifying-your-email-address`)).toBe(true)
})
})

it('disallows crawling of WIP localized content', async () => {
Object.values(languages)
.filter((language) => language.wip)
.forEach((language) => {
expect(allowIndex(`/${language.code}`)).toBe(false)
expect(allowIndex(`/${language.code}/articles/verifying-your-email-address`)).toBe(false)
})
Object.values(languages).forEach((language) => {
expect(allowIndex(`/${language.code}`)).toBe(true)
expect(allowIndex(`/${language.code}/articles/verifying-your-email-address`)).toBe(true)
})
})

it('disallows crawling of WIP products', async () => {
Expand Down
18 changes: 8 additions & 10 deletions tests/rendering/robots-txt.js
Original file line number Diff line number Diff line change
Expand Up @@ -25,16 +25,14 @@ describe('robots.txt', () => {
})

it('allows indexing of generally available localized content', async () => {
Object.values(languages)
.filter((language) => !language.wip)
.forEach((language) => {
expect(robots.isAllowed(`https://docs.github.com/${language.code}`)).toBe(true)
expect(
robots.isAllowed(
`https://docs.github.com/${language.code}/articles/verifying-your-email-address`
)
).toBe(true)
})
Object.values(languages).forEach((language) => {
expect(robots.isAllowed(`https://docs.github.com/${language.code}`)).toBe(true)
expect(
robots.isAllowed(
`https://docs.github.com/${language.code}/articles/verifying-your-email-address`
)
).toBe(true)
})
})

it('disallows indexing of azurecontainer.io domains', async () => {
Expand Down

0 comments on commit a1e3866

Please sign in to comment.