diff --git a/.changeset/large-dolls-juggle.md b/.changeset/large-dolls-juggle.md new file mode 100644 index 00000000000..9d0bef2b2ca --- /dev/null +++ b/.changeset/large-dolls-juggle.md @@ -0,0 +1,7 @@ +--- +"@primer/react": patch +--- + +`PageLayout` and `SplitPageLayout` are now SSR-compatible. + +Warning: In this new implementation, `PageLayout.Header` and `PageLayout.Footer` must be direct children of `PageLayout`. The same applies to `SplitPageLayout` diff --git a/examples/nextjs/package-lock.json b/examples/nextjs/package-lock.json index 425e6fee9bc..accbf490f2e 100644 --- a/examples/nextjs/package-lock.json +++ b/examples/nextjs/package-lock.json @@ -17,7 +17,7 @@ }, "../..": { "name": "@primer/react", - "version": "35.20.0", + "version": "35.21.0", "license": "MIT", "dependencies": { "@github/combobox-nav": "^2.1.5", @@ -26,7 +26,7 @@ "@github/relative-time-element": "^4.1.2", "@lit-labs/react": "1.1.1", "@primer/behaviors": "1.3.3", - "@primer/octicons-react": "^17.7.0", + "@primer/octicons-react": "^18.0.0", "@primer/primitives": "7.11.1", "@react-aria/ssr": "^3.1.0", "@styled-system/css": "^5.1.5", @@ -47,14 +47,14 @@ }, "devDependencies": { "@actions/core": "1.10.0", - "@babel/cli": "7.19.3", + "@babel/cli": "7.21.0", "@babel/core": "7.21.0", "@babel/eslint-parser": "7.19.1", "@babel/plugin-proposal-nullish-coalescing-operator": "7.18.6", - "@babel/plugin-proposal-optional-chaining": "7.16.7", - "@babel/plugin-transform-modules-commonjs": "7.19.6", + "@babel/plugin-proposal-optional-chaining": "7.21.0", + "@babel/plugin-transform-modules-commonjs": "7.21.2", "@babel/preset-react": "7.18.6", - "@babel/preset-typescript": "7.18.6", + "@babel/preset-typescript": "7.21.0", "@changesets/changelog-github": "0.4.8", "@changesets/cli": "2.26.0", "@github/markdownlint-github": "^0.3.0", @@ -67,7 +67,7 @@ "@rollup/plugin-terser": "0.4.0", "@rollup/plugin-typescript": "11.0.0", "@rollup/plugin-virtual": "3.0.1", - "@size-limit/preset-big-lib": "7.0.3", + "@size-limit/preset-big-lib": "8.2.4", "@storybook/addon-a11y": "6.5.14", "@storybook/addon-actions": "6.5.16", "@storybook/addon-essentials": "6.5.16", @@ -77,7 +77,7 @@ "@storybook/builder-webpack5": "6.5.16", "@storybook/jest": "0.0.10", "@storybook/manager-webpack5": "6.5.14", - "@storybook/react": "6.5.15", + "@storybook/react": "6.5.16", "@storybook/test-runner": "0.9.1", "@storybook/testing-library": "0.0.13", "@storybook/theming": "6.5.16", @@ -94,15 +94,16 @@ "@types/lodash.isobject": "3.0.7", "@types/lodash.keyby": "4.6.7", "@types/node": "16.11.11", - "@types/react": "18.0.26", + "@types/react": "18.0.28", "@types/react-dom": "18.0.9", - "@typescript-eslint/eslint-plugin": "4.33.0", - "@typescript-eslint/parser": "4.33.0", + "@typescript-eslint/eslint-plugin": "5.54.1", + "@typescript-eslint/parser": "5.54.1", "ajv": "8.12.0", "axe-core": "4.6.1", "babel-core": "7.0.0-bridge.0", "babel-loader": "^9.1.0", "babel-plugin-add-react-displayname": "0.0.5", + "babel-plugin-dev-expression": "0.2.3", "babel-plugin-macros": "3.1.0", "babel-plugin-open-source": "1.3.4", "babel-plugin-preval": "5.1.0", @@ -116,29 +117,30 @@ "concurrently": "7.6.0", "copyfiles": "2.4.1", "cross-env": "7.0.3", - "eslint": "7.32.0", - "eslint-plugin-github": "4.1.3", - "eslint-plugin-jest": "24.3.6", - "eslint-plugin-jsx-a11y": "6.6.1", - "eslint-plugin-mdx": "1.15.1", + "eslint": "8.35.0", + "eslint-import-resolver-typescript": "3.5.3", + "eslint-plugin-github": "4.6.1", + "eslint-plugin-jest": "27.2.1", + "eslint-plugin-jsx-a11y": "6.7.1", + "eslint-plugin-mdx": "2.0.5", "eslint-plugin-prettier": "4.2.1", - "eslint-plugin-primer-react": "0.7.4", + "eslint-plugin-primer-react": "1.0.1", "eslint-plugin-react": "7.32.2", "eslint-plugin-react-hooks": "4.6.0", - "eslint-plugin-storybook": "0.6.10", + "eslint-plugin-storybook": "0.6.11", "fast-glob": "3.2.12", "filesize": "10.0.6", "front-matter": "4.0.2", "gzip-size": "6.0.0", "husky": "8.0.2", "jest": "29.4.2", - "jest-axe": "5.0.1", + "jest-axe": "7.0.0", "jest-environment-jsdom": "29.4.3", "jest-fail-on-console": "3.0.2", "jest-matchmedia-mock": "1.1.0", "jest-styled-components": "6.3.4", "jest-watch-typeahead": "2.1.1", - "jscodeshift": "0.13.0", + "jscodeshift": "0.14.0", "lint-staged": "13.1.0", "lodash.groupby": "4.6.0", "lodash.isempty": "4.4.0", @@ -149,7 +151,7 @@ "mdast-util-from-markdown": "1.2.0", "mdast-util-frontmatter": "1.0.1", "mdast-util-mdx": "2.0.1", - "mdast-util-to-string": "3.1.0", + "mdast-util-to-string": "3.1.1", "micromark-extension-frontmatter": "1.0.0", "micromark-extension-mdxjs": "1.0.0", "prettier": "2.8.1", @@ -163,7 +165,7 @@ "rollup": "3.12.1", "rollup-plugin-visualizer": "5.9.0", "semver": "7.3.8", - "size-limit": "7.0.3", + "size-limit": "8.2.4", "storybook-addon-turbo-build": "1.1.0", "styled-components": "4.4.1", "terser": "5.16.1", @@ -173,7 +175,7 @@ "unist-util-find": "1.0.2", "unist-util-find-before": "3.0.0", "unist-util-flat-filter": "2.0.0", - "webpack": "5.74.0", + "webpack": "5.76.0", "yaml": "2.2.1" }, "engines": { @@ -1351,14 +1353,14 @@ "version": "file:../..", "requires": { "@actions/core": "1.10.0", - "@babel/cli": "7.19.3", + "@babel/cli": "7.21.0", "@babel/core": "7.21.0", "@babel/eslint-parser": "7.19.1", "@babel/plugin-proposal-nullish-coalescing-operator": "7.18.6", - "@babel/plugin-proposal-optional-chaining": "7.16.7", - "@babel/plugin-transform-modules-commonjs": "7.19.6", + "@babel/plugin-proposal-optional-chaining": "7.21.0", + "@babel/plugin-transform-modules-commonjs": "7.21.2", "@babel/preset-react": "7.18.6", - "@babel/preset-typescript": "7.18.6", + "@babel/preset-typescript": "7.21.0", "@changesets/changelog-github": "0.4.8", "@changesets/cli": "2.26.0", "@github/combobox-nav": "^2.1.5", @@ -1370,7 +1372,7 @@ "@lit-labs/react": "1.1.1", "@playwright/test": "1.30.0", "@primer/behaviors": "1.3.3", - "@primer/octicons-react": "^17.7.0", + "@primer/octicons-react": "^18.0.0", "@primer/primitives": "7.11.1", "@react-aria/ssr": "^3.1.0", "@rollup/plugin-babel": "6.0.3", @@ -1380,7 +1382,7 @@ "@rollup/plugin-terser": "0.4.0", "@rollup/plugin-typescript": "11.0.0", "@rollup/plugin-virtual": "3.0.1", - "@size-limit/preset-big-lib": "7.0.3", + "@size-limit/preset-big-lib": "8.2.4", "@storybook/addon-a11y": "6.5.14", "@storybook/addon-actions": "6.5.16", "@storybook/addon-essentials": "6.5.16", @@ -1390,7 +1392,7 @@ "@storybook/builder-webpack5": "6.5.16", "@storybook/jest": "0.0.10", "@storybook/manager-webpack5": "6.5.14", - "@storybook/react": "6.5.15", + "@storybook/react": "6.5.16", "@storybook/test-runner": "0.9.1", "@storybook/testing-library": "0.0.13", "@storybook/theming": "6.5.16", @@ -1410,19 +1412,20 @@ "@types/lodash.isobject": "3.0.7", "@types/lodash.keyby": "4.6.7", "@types/node": "16.11.11", - "@types/react": "18.0.26", + "@types/react": "18.0.28", "@types/react-dom": "18.0.9", "@types/styled-components": "^5.1.11", "@types/styled-system": "^5.1.12", "@types/styled-system__css": "^5.0.16", "@types/styled-system__theme-get": "^5.0.1", - "@typescript-eslint/eslint-plugin": "4.33.0", - "@typescript-eslint/parser": "4.33.0", + "@typescript-eslint/eslint-plugin": "5.54.1", + "@typescript-eslint/parser": "5.54.1", "ajv": "8.12.0", "axe-core": "4.6.1", "babel-core": "7.0.0-bridge.0", "babel-loader": "^9.1.0", "babel-plugin-add-react-displayname": "0.0.5", + "babel-plugin-dev-expression": "0.2.3", "babel-plugin-macros": "3.1.0", "babel-plugin-open-source": "1.3.4", "babel-plugin-preval": "5.1.0", @@ -1439,16 +1442,17 @@ "copyfiles": "2.4.1", "cross-env": "7.0.3", "deepmerge": "^4.2.2", - "eslint": "7.32.0", - "eslint-plugin-github": "4.1.3", - "eslint-plugin-jest": "24.3.6", - "eslint-plugin-jsx-a11y": "6.6.1", - "eslint-plugin-mdx": "1.15.1", + "eslint": "8.35.0", + "eslint-import-resolver-typescript": "3.5.3", + "eslint-plugin-github": "4.6.1", + "eslint-plugin-jest": "27.2.1", + "eslint-plugin-jsx-a11y": "6.7.1", + "eslint-plugin-mdx": "2.0.5", "eslint-plugin-prettier": "4.2.1", - "eslint-plugin-primer-react": "0.7.4", + "eslint-plugin-primer-react": "1.0.1", "eslint-plugin-react": "7.32.2", "eslint-plugin-react-hooks": "4.6.0", - "eslint-plugin-storybook": "0.6.10", + "eslint-plugin-storybook": "0.6.11", "fast-glob": "3.2.12", "filesize": "10.0.6", "focus-visible": "^5.2.0", @@ -1458,13 +1462,13 @@ "history": "^5.0.0", "husky": "8.0.2", "jest": "29.4.2", - "jest-axe": "5.0.1", + "jest-axe": "7.0.0", "jest-environment-jsdom": "29.4.3", "jest-fail-on-console": "3.0.2", "jest-matchmedia-mock": "1.1.0", "jest-styled-components": "6.3.4", "jest-watch-typeahead": "2.1.1", - "jscodeshift": "0.13.0", + "jscodeshift": "0.14.0", "lint-staged": "13.1.0", "lodash.groupby": "4.6.0", "lodash.isempty": "4.4.0", @@ -1475,7 +1479,7 @@ "mdast-util-from-markdown": "1.2.0", "mdast-util-frontmatter": "1.0.1", "mdast-util-mdx": "2.0.1", - "mdast-util-to-string": "3.1.0", + "mdast-util-to-string": "3.1.1", "micromark-extension-frontmatter": "1.0.0", "micromark-extension-mdxjs": "1.0.0", "prettier": "2.8.1", @@ -1490,7 +1494,7 @@ "rollup": "3.12.1", "rollup-plugin-visualizer": "5.9.0", "semver": "7.3.8", - "size-limit": "7.0.3", + "size-limit": "8.2.4", "storybook-addon-turbo-build": "1.1.0", "styled-components": "4.4.1", "styled-system": "^5.1.5", @@ -1501,7 +1505,7 @@ "unist-util-find": "1.0.2", "unist-util-find-before": "3.0.0", "unist-util-flat-filter": "2.0.0", - "webpack": "5.74.0", + "webpack": "5.76.0", "yaml": "2.2.1" } }, diff --git a/src/PageLayout/PageLayout.tsx b/src/PageLayout/PageLayout.tsx index f06af5ff006..e7efcb82cb4 100644 --- a/src/PageLayout/PageLayout.tsx +++ b/src/PageLayout/PageLayout.tsx @@ -4,15 +4,13 @@ import Box from '../Box' import {useId} from '../hooks/useId' import {useRefObjectAsForwardedRef} from '../hooks/useRefObjectAsForwardedRef' import {isResponsiveValue, ResponsiveValue, useResponsiveValue} from '../hooks/useResponsiveValue' +import {useSlots} from '../hooks/useSlots' import {BetterSystemStyleObject, merge, SxProp} from '../sx' import {Theme} from '../ThemeProvider' -import createSlots from '../utils/create-slots' import {canUseDOM} from '../utils/environment' import VisuallyHidden from '../_VisuallyHidden' import {useStickyPaneHeight} from './useStickyPaneHeight' -const {Slots, Slot} = createSlots(['Header', 'Footer']) - const REGION_ORDER = { header: 0, paneStart: 1, @@ -51,6 +49,9 @@ export type PageLayoutProps = { padding?: keyof typeof SPACING_MAP rowGap?: keyof typeof SPACING_MAP columnGap?: keyof typeof SPACING_MAP + + /** Private prop to allow SplitPageLayout to customize slot components */ + _slotsConfig?: Record<'header' | 'footer', React.ComponentType> } & SxProp const containerWidths = { @@ -68,9 +69,13 @@ const Root: React.FC> = ({ columnGap = 'normal', children, sx = {}, + _slotsConfig: slotsConfig, }) => { const {rootRef, enableStickyPane, disableStickyPane, contentTopRef, contentBottomRef, stickyPaneHeight} = useStickyPaneHeight() + + const [slots, rest] = useSlots(children, slotsConfig ?? {header: Header, footer: Footer}) + return ( > = ({ flexWrap: 'wrap', }} > - - {slots => { - return ( - <> - {slots.Header} - {children} - {slots.Footer} - - ) - }} - + {slots.header} + {rest} + {slots.footer} @@ -361,24 +358,22 @@ const Header: React.FC> = ({ const isHidden = useResponsiveValue(hidden, false) const {rowGap} = React.useContext(PageLayoutContext) return ( - - - + ) } @@ -826,25 +821,23 @@ const Footer: React.FC> = ({ const isHidden = useResponsiveValue(hidden, false) const {rowGap} = React.useContext(PageLayoutContext) return ( - - - + ) } diff --git a/src/PageLayout/__snapshots__/PageLayout.test.tsx.snap b/src/PageLayout/__snapshots__/PageLayout.test.tsx.snap index 59604aa94a9..97b5bfbea64 100644 --- a/src/PageLayout/__snapshots__/PageLayout.test.tsx.snap +++ b/src/PageLayout/__snapshots__/PageLayout.test.tsx.snap @@ -14,6 +14,10 @@ exports[`PageLayout renders condensed layout 1`] = ` flex-wrap: wrap; } +.c3 { + padding: 0; +} + .c5 { display: -webkit-box; display: -webkit-flex; @@ -72,14 +76,22 @@ exports[`PageLayout renders condensed layout 1`] = ` padding: 0; } -.c3 { - padding: 0; -} - .c0 { padding: 16px; } +.c2 { + width: 100%; + margin-bottom: 16px; +} + +.c4 { + margin-left: -16px; + margin-right: -16px; + display: none; + margin-top: 16px; +} + .c8 { display: -webkit-box; display: -webkit-flex; @@ -111,18 +123,6 @@ exports[`PageLayout renders condensed layout 1`] = ` margin-right: 16px; } -.c2 { - width: 100%; - margin-bottom: 16px; -} - -.c4 { - margin-left: -16px; - margin-right: -16px; - display: none; - margin-top: 16px; -} - .c12 { -webkit-order: 4; -ms-flex-order: 4; @@ -150,6 +150,13 @@ exports[`PageLayout renders condensed layout 1`] = ` } } +@media screen and (min-width:768px) { + .c4 { + margin-left: 0 !important; + margin-right: 0 !important; + } +} + @media screen and (min-width:768px) { .c8 { width: auto; @@ -169,13 +176,6 @@ exports[`PageLayout renders condensed layout 1`] = ` } } -@media screen and (min-width:768px) { - .c4 { - margin-left: 0 !important; - margin-right: 0 !important; - } -} -
> = props => { - return + return ( + + ) } Root.displayName = 'SplitPageLayout' diff --git a/src/SplitPageLayout/__snapshots__/SplitPageLayout.test.tsx.snap b/src/SplitPageLayout/__snapshots__/SplitPageLayout.test.tsx.snap index 321c53c4503..c35d5dc97cd 100644 --- a/src/SplitPageLayout/__snapshots__/SplitPageLayout.test.tsx.snap +++ b/src/SplitPageLayout/__snapshots__/SplitPageLayout.test.tsx.snap @@ -18,6 +18,24 @@ exports[`SplitPageLayout renders default layout 1`] = ` flex-wrap: wrap; } +.c2 { + width: 100%; + margin-bottom: 0; +} + +.c3 { + padding: 16px; +} + +.c4 { + margin-left: 0; + margin-right: 0; + display: block; + height: 1px; + background-color: #d0d7de; + margin-top: 0; +} + .c5 { display: -webkit-box; display: -webkit-flex; @@ -107,24 +125,6 @@ exports[`SplitPageLayout renders default layout 1`] = ` padding: 16px; } -.c2 { - width: 100%; - margin-bottom: 0; -} - -.c3 { - padding: 16px; -} - -.c4 { - margin-left: 0; - margin-right: 0; - display: block; - height: 1px; - background-color: #d0d7de; - margin-top: 0; -} - .c12 { -webkit-order: 4; -ms-flex-order: 4; @@ -142,6 +142,19 @@ exports[`SplitPageLayout renders default layout 1`] = ` margin-bottom: 0; } +@media screen and (min-width:1012px) { + .c3 { + padding: 24px; + } +} + +@media screen and (min-width:768px) { + .c4 { + margin-left: 0 !important; + margin-right: 0 !important; + } +} + @media screen and (min-width:1012px) { .c7 { padding: 24px; @@ -191,19 +204,6 @@ exports[`SplitPageLayout renders default layout 1`] = ` } } -@media screen and (min-width:1012px) { - .c3 { - padding: 24px; - } -} - -@media screen and (min-width:768px) { - .c4 { - margin-left: 0 !important; - margin-right: 0 !important; - } -} - @media screen and (min-width:768px) { .c13 { margin-left: 0 !important; diff --git a/src/hooks/__tests__/useSlots.test.tsx b/src/hooks/__tests__/useSlots.test.tsx new file mode 100644 index 00000000000..d491f90c475 --- /dev/null +++ b/src/hooks/__tests__/useSlots.test.tsx @@ -0,0 +1,116 @@ +import {renderHook} from '@testing-library/react-hooks' +import React from 'react' +import {useSlots} from '../useSlots' + +function TestComponentA(props: React.PropsWithChildren) { + return
+} + +function TestComponentB(props: React.PropsWithChildren) { + return
+} + +test('extracts elements based on config object', () => { + const children = [, ,
Hello World
] + const slotsConfig = { + a: TestComponentA, + b: TestComponentB, + } + const {result} = renderHook(() => useSlots(children, slotsConfig)) + expect(result.current).toMatchInlineSnapshot(` + [ + { + "a": , + "b": , + }, + [ +
+ Hello World +
, + ], + ] + `) +}) + +test('handles empty config object', () => { + const children = [, ,
Hello World
] + const slotsConfig = {} + const {result} = renderHook(() => useSlots(children, slotsConfig)) + expect(result.current).toMatchInlineSnapshot(` + [ + {}, + [ + , + , +
+ Hello World +
, + ], + ] + `) +}) + +test('handles empty children', () => { + const children: React.ReactNode = [] + const slotsConfig = { + a: TestComponentA, + b: TestComponentB, + } + const {result} = renderHook(() => useSlots(children, slotsConfig)) + expect(result.current).toMatchInlineSnapshot(` + [ + { + "a": undefined, + "b": undefined, + }, + [], + ] + `) +}) + +test('ignores nested slots', () => { + const children = [ + , +
+ +
, + ] + const slotsConfig = { + a: TestComponentA, + b: TestComponentB, + } + const {result} = renderHook(() => useSlots(children, slotsConfig)) + expect(result.current).toMatchInlineSnapshot(` + [ + { + "a": , + "b": undefined, + }, + [ +
+ +
, + ], + ] + `) +}) + +test('warns about duplicate slots', () => { + const warnSpy = jest.spyOn(console, 'warn').mockImplementation(() => {}) + const children = [A1, A2] + const slotsConfig = { + a: TestComponentA, + } + const {result} = renderHook(() => useSlots(children, slotsConfig)) + expect(result.current).toMatchInlineSnapshot(` + [ + { + "a": + A1 + , + }, + [], + ] + `) + expect(warnSpy).toHaveBeenCalledTimes(1) +}) diff --git a/src/hooks/useSlots.ts b/src/hooks/useSlots.ts new file mode 100644 index 00000000000..6c1de73a3dd --- /dev/null +++ b/src/hooks/useSlots.ts @@ -0,0 +1,66 @@ +import React from 'react' +import {warning} from '../utils/warning' + +export type SlotConfig = Record + +type SlotElements = { + [Property in keyof Type]: React.ReactElement +} + +/** + * Extract components from `children` so we can render them in different places, + * allowing us to implement components with SSR-compatible slot APIs. + * Note: We can only extract direct children, not nested ones. + */ +export function useSlots( + children: React.ReactNode, + config: T, +): [Partial>, React.ReactNode[]] { + // Object mapping slot names to their elements + const slots: Partial> = mapValues(config, () => undefined) + + // Array of elements that are not slots + const rest: React.ReactNode[] = [] + + const keys = Object.keys(config) as Array + const values = Object.values(config) + + // eslint-disable-next-line github/array-foreach + React.Children.forEach(children, child => { + if (!React.isValidElement(child)) { + rest.push(child) + return + } + + const index = values.findIndex(value => { + return child.type === value + }) + + // If the child is not a slot, add it to the `rest` array + if (index === -1) { + rest.push(child) + return + } + + const slotKey = keys[index] + + // If slot is already filled, ignore duplicates + if (slots[slotKey]) { + warning(true, `Found duplicate "${String(slotKey)}" slot. Only the first will be rendered.`) + return + } + + // If the child is a slot, add it to the `slots` object + slots[slotKey] = child + }) + + return [slots, rest] +} + +/** Map the values of an object */ +function mapValues, V>(obj: T, fn: (value: T[keyof T]) => V) { + return Object.keys(obj).reduce((result, key: keyof T) => { + result[key] = fn(obj[key]) + return result + }, {} as Record) +}