Skip to content

Commit

Permalink
Fix Set wrapper type so you can have an array of wrappers (#9314)
Browse files Browse the repository at this point in the history
  • Loading branch information
Tobbe committed Oct 19, 2023
1 parent fca02b6 commit c44a524
Show file tree
Hide file tree
Showing 10 changed files with 135 additions and 61 deletions.
43 changes: 24 additions & 19 deletions packages/router/src/Set.tsx
Original file line number Diff line number Diff line change
@@ -1,15 +1,19 @@
import type { ReactElement, ReactNode } from 'react'
import * as React from 'react'
import React from 'react'

export type WrapperType<WTProps> = (
props: WTProps & { children: ReactNode }
props: Omit<WTProps, 'wrap' | 'children'> & {
children: ReactNode
}
) => ReactElement | null

type SetProps<P> = P & {
// P is the interface for the props that are forwarded to the wrapper
// components. TypeScript will most likely infer this for you, but if you
// need to you can specify it yourself in your JSX like so:
// <Set<{theme: string}> wrap={ThemeableLayout} theme="dark">
/**
* P is the interface for the props that are forwarded to the wrapper
* components. TypeScript will most likely infer this for you, but if you
* need to you can specify it yourself in your JSX like so:
* <Set<{theme: string}> wrap={ThemableLayout} theme="dark">
*/
wrap?: WrapperType<P> | WrapperType<P>[]
/**
*`Routes` nested in a `<Set>` with `private` specified require
Expand All @@ -19,19 +23,23 @@ type SetProps<P> = P & {
* @deprecated Please use `<PrivateSet>` instead
*/
private?: boolean
/** The page name where a user will be redirected when not authenticated */
/**
* The page name where a user will be redirected when not authenticated
*
* @deprecated Please use `<PrivateSet>` instead and specify this prop there
*/
unauthenticated?: string
/**
* Route is permitted when authenticated and use has any of the provided
* Route is permitted when authenticated and user has any of the provided
* roles such as "admin" or ["admin", "editor"]
*/
roles?: string | string[]
/** Prerender all pages in the set */
prerender?: boolean
children: ReactNode
/** Loading state for auth to distinguish with whileLoading */
whileLoadingAuth?: () => React.ReactElement | null
whileLoadingPage?: () => React.ReactElement | null
whileLoadingAuth?: () => ReactElement | null
whileLoadingPage?: () => ReactElement | null
}

/**
Expand All @@ -40,11 +48,10 @@ type SetProps<P> = P & {
* JSX like so:
* <Set<{theme: string}> wrap={ThemeableLayout} theme="dark">
*/
export function Set<WrapperProps>(_props: SetProps<WrapperProps>) {
export function Set<WrapperProps>(props: SetProps<WrapperProps>) {
// @MARK: Virtual Component, this is actually never rendered
// See analyzeRoutes in utils.tsx, inside the isSetNode block

return null
return <>{props.children}</>
}

type PrivateSetProps<P> = Omit<
Expand All @@ -57,18 +64,16 @@ type PrivateSetProps<P> = Omit<
}

/** @deprecated Please use `<PrivateSet>` instead */
export function Private<WrapperProps>(_props: PrivateSetProps<WrapperProps>) {
export function Private<WrapperProps>(props: PrivateSetProps<WrapperProps>) {
// @MARK Virtual Component, this is actually never rendered
// See analyzeRoutes in utils.tsx, inside the isSetNode block
return null
return <>{props.children}</>
}

export function PrivateSet<WrapperProps>(
_props: PrivateSetProps<WrapperProps>
) {
export function PrivateSet<WrapperProps>(props: PrivateSetProps<WrapperProps>) {
// @MARK Virtual Component, this is actually never rendered
// See analyzeRoutes in utils.tsx, inside the isSetNode block
return null
return <>{props.children}</>
}

export const isSetNode = (
Expand Down
16 changes: 13 additions & 3 deletions packages/router/src/__tests__/analyzeRoutes.test.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -6,9 +6,19 @@ import { analyzeRoutes } from '../util'

const FakePage = () => <h1>Fake Page</h1>

const FakeLayout1 = ({ children }) => <div className="layout1">{children}</div>
const FakeLayout2 = ({ children }) => <div className="layout2">{children}</div>
const FakeLayout3 = ({ children }) => <div className="layout2">{children}</div>
interface LayoutProps {
children: React.ReactNode
}

const FakeLayout1 = ({ children }: LayoutProps) => (
<div className="layout1">{children}</div>
)
const FakeLayout2 = ({ children }: LayoutProps) => (
<div className="layout2">{children}</div>
)
const FakeLayout3 = ({ children }: LayoutProps) => (
<div className="layout2">{children}</div>
)

describe('AnalyzeRoutes: with homePage and Children', () => {
const CheckRoutes = (
Expand Down
7 changes: 6 additions & 1 deletion packages/router/src/__tests__/links.test.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,9 @@ import React from 'react'

import { toHaveClass, toHaveStyle } from '@testing-library/jest-dom/matchers'
import { render } from '@testing-library/react'

// TODO: Remove when jest configs are in place
// @ts-expect-error - Issue with TS and jest-dom
expect.extend({ toHaveClass, toHaveStyle })

import { NavLink, useMatch, Link } from '../links'
Expand Down Expand Up @@ -284,7 +286,10 @@ describe('<NavLink />', () => {
})

describe('useMatch', () => {
const MyLink = ({ to, ...rest }) => {
const MyLink = ({
to,
...rest
}: React.ComponentPropsWithoutRef<typeof Link>) => {
const [pathname, queryString] = to.split('?')
const matchInfo = useMatch(pathname, {
searchParams: flattenSearchParams(queryString),
Expand Down
5 changes: 3 additions & 2 deletions packages/router/src/__tests__/nestedSets.test.tsx
Original file line number Diff line number Diff line change
@@ -1,4 +1,5 @@
import React from 'react'
import type { ReactNode } from 'react'

import '@testing-library/jest-dom/extend-expect'
import { act, render } from '@testing-library/react'
Expand Down Expand Up @@ -33,7 +34,7 @@ afterAll(() => {
})

test('Sets nested in Private should not error out if no authenticated prop provided', () => {
const Layout1 = ({ children }) => (
const Layout1 = ({ children }: { children: ReactNode }) => (
<div>
<p>Layout1</p>
{children}
Expand Down Expand Up @@ -74,7 +75,7 @@ test('Sets nested in Private should not error out if no authenticated prop provi
})

test('Sets nested in `<Set private>` should not error out if no authenticated prop provided', () => {
const Layout1 = ({ children }) => (
const Layout1 = ({ children }: { children: ReactNode }) => (
<div>
<p>Layout1</p>
{children}
Expand Down
4 changes: 4 additions & 0 deletions packages/router/src/__tests__/route-announcer.test.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -51,6 +51,7 @@ const EmptyH1Page = () => (

beforeEach(() => {
window.history.pushState({}, '', '/')
// @ts-expect-error - No type gen here for routes like there is in a real app
Object.keys(routes).forEach((key) => delete routes[key])
})

Expand Down Expand Up @@ -94,6 +95,7 @@ test('gets the announcement in the correct order of priority', async () => {

// navigate to h1
// since there's no RouteAnnouncement, it should announce the h1.
// @ts-expect-error - No type gen here for routes like there is in a real app
act(() => navigate(routes.h1()))
await waitFor(() => {
screen.getByText(/H1 Page/i)
Expand All @@ -102,6 +104,7 @@ test('gets the announcement in the correct order of priority', async () => {

// navigate to noH1.
// since there's no h1, it should announce the title.
// @ts-expect-error - No type gen here for routes like there is in a real app
act(() => navigate(routes.noH1()))
await waitFor(() => {
screen.getByText(/NoH1 Page/i)
Expand All @@ -113,6 +116,7 @@ test('gets the announcement in the correct order of priority', async () => {
// navigate to noH1OrTitle.
// since there's no h1 or title,
// it should announce the location.
// @ts-expect-error - No type gen here for routes like there is in a real app
act(() => navigate(routes.noH1OrTitle()))
await waitFor(() => {
screen.getByText(/NoH1OrTitle Page/i)
Expand Down
4 changes: 3 additions & 1 deletion packages/router/src/__tests__/route-focus.test.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,7 @@ const NoRouteFocusPage = () => <h1>No Route Focus Page</h1>

const RouteFocusNoChildren = () => (
<>
{/* @ts-expect-error - Testing a JS scenario */}
<RouteFocus></RouteFocus>
<h1>Route Focus No Children Page</h1>
<p></p>
Expand All @@ -45,7 +46,8 @@ const RouteFocusNegativeTabIndexPage = () => (
)

beforeEach(() => {
window.history.pushState({}, null, '/')
window.history.pushState({}, '', '/')
// @ts-expect-error - No type gen here for routes like there is in a real app
Object.keys(routes).forEach((key) => delete routes[key])
})

Expand Down
Loading

0 comments on commit c44a524

Please sign in to comment.