Merge pull request #31116 from github/repo-sync

Repo sync

.github/workflows/local-dev.yml

@@ -21,7 +21,6 @@ jobs:
       - name: Check out repo
         uses: actions/checkout@3df4ab11eba7bda6032a0b82a6bb43b11571feac # v4.0.0
 
-      # - uses: ./.github/actions/node-npm-setup
       - name: Install dependencies
         run: npm install
 
@@ -31,6 +30,20 @@ jobs:
       - name: Disable Next.js telemetry
         run: npx next telemetry disable
 
+      # The Playwright test, with the env vars we set here, takes care of
+      # starting a server and shutting it down when it's done.
+      # That's why it's important this step comes before the `npm start &`
+      # step below.
+      - name: Run Playwright tests
+        env:
+          # This is what local dev contributors are expected to do.
+          PLAYWRIGHT_START_SERVER_COMMAND: 'npm start'
+          # This is so that timeouts aren't retried, which can lead to
+          # tests not exiting at the end with a non-zero. Otherwise,
+          # by default failures are marked as "flaky" instead of "failed".
+          PLAYWRIGHT_RETRIES: 0
+        run: npm run playwright-test -- playwright-local-dev
+
       - name: Start server in the background
         run: npm start > /tmp/stdout.log 2> /tmp/stderr.log &
 

README.md

@@ -4,23 +4,23 @@ This repository contains the documentation website code and Markdown source file
 
 GitHub's Docs team works on pre-production content in a private repo that regularly syncs with this public repo.
 
-Use the table of contents icon <img src="/contributing/images/table-of-contents.png" width="25" height="25" /> on the top left corner of this document to navigate to a specific section quickly.
+Use the table of contents icon <img alt="Table of contents icon" src="./contributing/images/table-of-contents.png" width="25" height="25" /> on the top left corner of this document to navigate to a specific section quickly.
 
 ## Contributing
 
 We accept different types of contributions, including some that don't require you to write a single line of code. For detailed instructions on how to get started with our project, see "[About contributing to GitHub Docs](https://docs.github.com/en/contributing/collaborating-on-github-docs/about-contributing-to-github-docs)."
 
-
 ### Ways to contribute
+
 On the GitHub Docs site, you can contribute by clicking the **Make a contribution** button at the bottom of the page to open a pull request for quick fixes like typos, updates, or link fixes.
 
 You can also contribute by creating a local environment or opening a Codespace. For more information, see "[Setting up your environment to work on GitHub Docs](https://docs.github.com/en/contributing/setting-up-your-environment-to-work-on-github-docs)."
 
-<img src="./contributing/images/contribution_cta.png" width="400">
+<img alt="Contribution call-to-action" src="./contributing/images/contribution_cta.png" width="400">
 
 For more complex contributions, please open an issue using the most appropriate [issue template](https://github.com/github/docs/issues/new/choose) to describe the changes you'd like to see.
 
-If you're looking for a way to contribute, you can scan through our [help wanted board](https://github.com/github/docs/issues?q=is%3Aopen+is%3Aissue+label%3A%22help+wanted%22) to find open issues already approved for work. 
+If you're looking for a way to contribute, you can scan through our [help wanted board](https://github.com/github/docs/issues?q=is%3Aopen+is%3Aissue+label%3A%22help+wanted%22) to find open issues already approved for work.
 
 ### Join us in discussions
 

src/fixtures/playwright.config.ts

@@ -6,6 +6,15 @@ import { defineConfig, devices } from '@playwright/test'
  */
 // require('dotenv').config();
 
+const PLAYWRIGHT_START_SERVER_COMMAND =
+  process.env.PLAYWRIGHT_START_SERVER_COMMAND || 'npm run start-for-playwright'
+
+const RETRIES = process.env.PLAYWRIGHT_RETRIES
+  ? Number(process.env.PLAYWRIGHT_RETRIES)
+  : process.env.CI
+    ? 2
+    : 0
+
 /**
  * See https://playwright.dev/docs/test-configuration.
  */
@@ -25,7 +34,7 @@ export default defineConfig({
   /* Fail the build on CI if you accidentally left test.only in the source code. */
   forbidOnly: !!process.env.CI,
   /* Retry on CI only */
-  retries: process.env.CI ? 2 : 0,
+  retries: RETRIES,
   /* Opt out of parallel tests on CI. */
   workers: process.env.PLAYWRIGHT_WORKERS
     ? JSON.parse(process.env.PLAYWRIGHT_WORKERS)
@@ -102,7 +111,7 @@ export default defineConfig({
 
   /* Run your local dev server before starting the tests */
   webServer: {
-    command: 'npm run start-for-playwright',
+    command: PLAYWRIGHT_START_SERVER_COMMAND,
     port: 4000,
   },
 })

src/fixtures/tests/playwright-local-dev.spec.ts

@@ -0,0 +1,34 @@
+/**
+ * These tests assume you have starte the local dev server as a contributor
+ * would. It does *not* use fixture data. It uses real English content
+ * as seen in `main` or in the current branch. Therefore be careful
+ * with what you can expect to find. Stick to known and stable content.
+ *
+ * It's always a risk that the content changes and can break tests
+ * that exist to test the *code*. But these tests are ultimately there to
+ * do what a human would do which is: Start the server, then open the
+ * browser, then click around, then search, etc.
+ *
+ */
+
+import { test, expect } from '@playwright/test'
+
+test('view home page', async ({ page }) => {
+  await page.goto('/')
+  await expect(page).toHaveTitle(/GitHub Docs/)
+})
+
+test('click "Get started" from home page', async ({ page }) => {
+  await page.goto('/')
+  await page.getByRole('link', { name: 'Get started' }).click()
+  await expect(page).toHaveTitle(/Get started with GitHub/)
+  await expect(page).toHaveURL(/\/en\/get-started/)
+})
+
+test('search "git" and get results', async ({ page }) => {
+  await page.goto('/')
+  await page.getByTestId('site-search-input').click()
+  await page.getByTestId('site-search-input').fill('git')
+  await page.getByTestId('site-search-input').press('Enter')
+  await expect(page.getByRole('heading', { name: /\d+ Search results for "git"/ })).toBeVisible()
+})

src/frame/components/DefaultLayout.tsx

@@ -17,8 +17,8 @@ const MINIMAL_RENDER = Boolean(JSON.parse(process.env.MINIMAL_RENDER || 'false')
 
 type Props = { children?: React.ReactNode }
 export const DefaultLayout = (props: Props) => {
+  const mainContext = useMainContext()
   const {
-    page,
     error,
     isHomepageVersion,
     currentPathWithoutLanguage,
@@ -27,10 +27,10 @@ export const DefaultLayout = (props: Props) => {
     relativePath,
     fullUrl,
     status,
-  } = useMainContext()
+  } = mainContext
+  const page = mainContext.page!
   const { t } = useTranslation(['meta', 'scroll_button'])
   const router = useRouter()
-  const metaDescription = page.introPlainText ? page.introPlainText : t('default_description')
   const { languages } = useLanguages()
 
   // This is only true when we do search indexing which renders every page
@@ -55,6 +55,8 @@ export const DefaultLayout = (props: Props) => {
     )
   }
 
+  const metaDescription = page.introPlainText ? page.introPlainText : t('default_description')
+
   return (
     <>
       <Head>

src/frame/components/context/MainContext.tsx

@@ -97,7 +97,7 @@ export type MainContextT = {
     href: string
   }
   currentProduct?: ProductT
-  currentLayoutName: string
+  currentLayoutName?: string
   isHomepageVersion: boolean
   data: DataT
   error: string
@@ -120,7 +120,7 @@ export type MainContextT = {
     hidden: boolean
     noEarlyAccessBanner: boolean
     applicableVersions: string[]
-  }
+  } | null
 
   enterpriseServerVersions: Array<string>
 
@@ -172,10 +172,9 @@ export const getMainContext = async (req: any, res: any): Promise<MainContextT>
     delete req.context.site.data.ui.ms
   }
 
-  if (!req.context.page) {
-    throw new Error(`No page context (${req.url})`)
-  }
-  const { documentType } = req.context.page
+  const { page } = req.context
+
+  const documentType = page ? (page.documentType as string) : undefined
 
   const ui: UIStrings = {}
   addUINamespaces(req, ui, DEFAULT_UI_NAMESPACES)
@@ -210,11 +209,24 @@ export const getMainContext = async (req: any, res: any): Promise<MainContextT>
   // as a full version string if the release candidate is set.
   const releaseCandidateVersion = releaseCandidate ? `enterprise-server@${releaseCandidate}` : null
 
-  return {
+  const pageInfo =
+    (page && {
+      documentType,
+      type: req.context.page.type || null,
+      title: req.context.page.title,
+      fullTitle: req.context.page.fullTitle || null,
+      topics: req.context.page.topics || [],
+      introPlainText: req.context.page?.introPlainText || null,
+      applicableVersions: req.context.page?.permalinks.map((obj: any) => obj.pageVersion) || [],
+      hidden: req.context.page.hidden || false,
+      noEarlyAccessBanner: req.context.page.noEarlyAccessBanner || false,
+    }) ||
+    null
+
+  const props: MainContextT = {
     breadcrumbs: req.context.breadcrumbs || {},
     communityRedirect: req.context.page?.communityRedirect || {},
     currentProduct: req.context.productMap[req.context.currentProduct] || null,
-    currentLayoutName: req.context.currentLayoutName,
     isHomepageVersion: req.context.page?.documentType === 'homepage',
     error: req.context.error ? req.context.error.toString() : '',
     data: {
@@ -230,18 +242,7 @@ export const getMainContext = async (req: any, res: any): Promise<MainContextT>
     },
     currentCategory: req.context.currentCategory || '',
     currentPathWithoutLanguage: req.context.currentPathWithoutLanguage,
-    relativePath: req.context.page?.relativePath,
-    page: {
-      documentType,
-      type: req.context.page.type || null,
-      title: req.context.page.title,
-      fullTitle: req.context.page.fullTitle,
-      topics: req.context.page.topics || [],
-      introPlainText: req.context.page?.introPlainText,
-      applicableVersions: req.context.page?.permalinks.map((obj: any) => obj.pageVersion) || [],
-      hidden: req.context.page.hidden || false,
-      noEarlyAccessBanner: req.context.page.noEarlyAccessBanner || false,
-    },
+    page: pageInfo,
     enterpriseServerReleases: pick(req.context.enterpriseServerReleases, [
       'isOldestReleaseDeprecated',
       'oldestSupported',
@@ -267,6 +268,14 @@ export const getMainContext = async (req: any, res: any): Promise<MainContextT>
     status: res.statusCode,
     fullUrl: req.protocol + '://' + req.get('host') + req.originalUrl,
   }
+
+  if (req.context.currentLayoutName) {
+    props.currentLayoutName = req.context.currentLayoutName
+  }
+  if (req.context.page?.relativePath) {
+    props.relativePath = req.context.page.relativePath
+  }
+  return props
 }
 
 export const MainContext = createContext<MainContextT | null>(null)

src/frame/components/page-header/HeaderNotifications.tsx

@@ -25,7 +25,9 @@ type Notif = {
 export const HeaderNotifications = () => {
   const router = useRouter()
   const { currentVersion } = useVersion()
-  const { relativePath, allVersions, data, currentPathWithoutLanguage, page } = useMainContext()
+  const mainContext = useMainContext()
+  const { relativePath, allVersions, data, currentPathWithoutLanguage } = mainContext
+  const page = mainContext.page!
   const { userLanguage, setUserLanguageCookie } = useUserLanguage()
   const { languages } = useLanguages()
 

src/frame/middleware/next.js

@@ -8,24 +8,14 @@ export const nextHandleRequest = nextApp.getRequestHandler()
 await nextApp.prepare()
 
 function renderPageWithNext(req, res, next) {
-  const isNextDataRequest = req.path.startsWith('/_next') && !req.path.startsWith('/_next/data')
-
-  if (
-    isNextDataRequest &&
-    // In local development, the very first request for a _next/static file
-    // triggers Nextjs to build it. So we need to let Nextjs handle that.
-    // But once it's built, we can handle it ourselves.
-    !req.path.startsWith('/_next/webpack-hmr') &&
-    // If the file doesn't exist on disk, and fell through our express.static
-    // for the `_next/static` prefix, it means the file does not exist.
-    // And trying to handle it will trigger the run of
-    // getServerSideProps() in `pages/index.tsx` which assumes there exists
-    // a page always.
-    !/_next\/static\/webpack\/[a-f0-9]+\.webpack\.hot-update\.json/.test(req.path)
-  ) {
+  if (req.path.startsWith('/_next') && !req.path.startsWith('/_next/data')) {
     return nextHandleRequest(req, res)
   }
 
+  // Note that URLs like `/_next/webpack-hmr` and
+  // '/_next/static/webpack/64e44ef62e261d3a.webpack.hot-update.json' has to
+  // go through here.
+
   return next()
 }
 

src/graphql/pages/explorer.tsx

@@ -9,7 +9,11 @@ type Props = {
   graphqlExplorerUrl: string
 }
 export default function GQLExplorer({ mainContext, graphqlExplorerUrl }: Props) {
-  const { page } = mainContext
+  // Use TypeScript's "not null assertion" because `context.page` should
+  // will present in main context if it's gotten to the stage of React
+  // rendering.
+  const page = mainContext.page!
+
   const graphiqlRef = useRef<HTMLIFrameElement>(null)
 
   useEffect(() => {

src/landings/components/ArticleList.tsx

@@ -22,7 +22,12 @@ export const ArticleList = ({
   articles,
 }: ArticleListPropsT) => {
   const { t } = useTranslation('product_landing')
-  const { page } = useMainContext()
+  const mainContext = useMainContext()
+  // Use TypeScript's "not null assertion" because `mainContext.page` should
+  // will present in mainContext if it's gotten to the stage of React
+  // rendering.
+  const page = mainContext.page!
+
   return (
     <>
       {title && (

src/landings/pages/product.tsx

@@ -99,7 +99,12 @@ const GlobalPage = ({
       </ArticleContext.Provider>
     )
   } else {
-    throw new Error('No context provided to page')
+    // In local dev, when Next.js needs the initial compiled version
+    // it will request `/_next/static/webpack/$HASH.webpack.hot-update.json`
+    // and then we just let the `content` be undefined.
+    if (!router.asPath.startsWith('/_next/static/')) {
+      throw new Error('No context provided to page')
+    }
   }
 
   return <MainContext.Provider value={mainContext}>{content}</MainContext.Provider>
@@ -130,7 +135,7 @@ export const getServerSideProps: GetServerSideProps<Props> = async (context) =>
     if (props.tocLandingContext.currentLearningTrack?.trackName) {
       additionalUINamespaces.push('learning_track_nav')
     }
-  } else {
+  } else if (props.mainContext.page) {
     // All articles that might have hover cards needs this
     additionalUINamespaces.push('popovers')
 

src/search/components/NoQuery.tsx

@@ -5,7 +5,11 @@ import { useTranslation } from 'src/languages/components/useTranslation'
 
 export function NoQuery() {
   const { t } = useTranslation(['search'])
-  const { page } = useMainContext()
+  const mainContext = useMainContext()
+  // Use TypeScript's "not null assertion" because `context.page` should
+  // will present in main context if it's gotten to the stage of React
+  // rendering.
+  const page = mainContext.page!
 
   return (
     <>

src/search/components/index.tsx

@@ -30,7 +30,9 @@ export function Search({ search }: Props) {
   const { results, validationErrors } = search
   const hasQuery = Boolean((query && query.trim()) || '')
 
-  let pageTitle = documentPage.fullTitle
+  // Mostly to satisfy TypeScript because the useMainContext hook
+  // is run on every request and every request doesn't have a page.
+  let pageTitle = documentPage?.fullTitle || 'Search'
   if (hasQuery) {
     pageTitle = `${t('search_results_for')} "${query.trim()}"`
     if (currentVersion !== DEFAULT_VERSION) {

src/versions/components/VersionPicker.tsx

@@ -15,7 +15,12 @@ type Props = {
 export const VersionPicker = ({ xs }: Props) => {
   const router = useRouter()
   const { currentVersion } = useVersion()
-  const { allVersions, page, enterpriseServerVersions } = useMainContext()
+  const mainContext = useMainContext()
+  // Use TypeScript's "not null assertion" because  mainContext.page` should
+  // will present in mainContext if it's gotten to the stage of React
+  // rendering.
+  const page = mainContext.page!
+  const { allVersions, enterpriseServerVersions } = mainContext
   const { t } = useTranslation(['pages', 'picker'])
 
   if (page.applicableVersions && page.applicableVersions.length < 1) {