Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

feat(react): add experimental wrap to remove unnecessary hocs #270

Merged
merged 20 commits into from
Nov 9, 2023
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
5 changes: 5 additions & 0 deletions .changeset/pretty-pumpkins-jam.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
---
"@suspensive/react": minor
---

feat(react): add experimental wrap to remove unnecessary hocs' implementation
35 changes: 5 additions & 30 deletions packages/react/src/AsyncBoundary.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@ import { ErrorBoundary } from './ErrorBoundary'
import type { ErrorBoundaryProps } from './ErrorBoundary'
import { Suspense } from './Suspense'
import type { PropsWithoutChildren } from './types'
import { wrap } from './wrap'

export type AsyncBoundaryProps = Omit<SuspenseProps, 'fallback'> &
Omit<ErrorBoundaryProps, 'fallback'> & {
Expand Down Expand Up @@ -46,36 +47,10 @@ export const AsyncBoundary = BaseAsyncBoundary as typeof BaseAsyncBoundary & {
AsyncBoundary.CSROnly = CSROnlyAsyncBoundary

export const withAsyncBoundary = <TProps extends ComponentProps<ComponentType> = Record<string, never>>(
Component: ComponentType<TProps>,
component: ComponentType<TProps>,
asyncBoundaryProps: PropsWithoutChildren<AsyncBoundaryProps>
) => {
const Wrapped = (props: TProps) => (
<AsyncBoundary {...asyncBoundaryProps}>
<Component {...props} />
</AsyncBoundary>
)

if (process.env.NODE_ENV !== 'production') {
const name = Component.displayName || Component.name || 'Component'
Wrapped.displayName = `withAsyncBoundary(${name})`
}

return Wrapped
}
Comment on lines -51 to -64
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Unnecessary implementations of all hocs is removed by wrap function

) => wrap.AsyncBoundary(asyncBoundaryProps).on(component)
withAsyncBoundary.CSROnly = <TProps extends ComponentProps<ComponentType> = Record<string, never>>(
Component: ComponentType<TProps>,
component: ComponentType<TProps>,
asyncBoundaryProps: PropsWithoutChildren<AsyncBoundaryProps>
) => {
const Wrapped = (props: TProps) => (
<AsyncBoundary.CSROnly {...asyncBoundaryProps}>
<Component {...props} />
</AsyncBoundary.CSROnly>
)

if (process.env.NODE_ENV !== 'production') {
const name = Component.displayName || Component.name || 'Component'
Wrapped.displayName = `withAsyncBoundary.CSROnly(${name})`
}

return Wrapped
}
) => wrap.AsyncBoundary.CSROnly(asyncBoundaryProps).on(component)
23 changes: 7 additions & 16 deletions packages/react/src/Delay.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@ import type { ComponentProps, ComponentType, PropsWithChildren } from 'react'
import { createContext, useContext, useState } from 'react'
import { useTimeout } from './hooks'
import type { PropsWithoutChildren } from './types'
import { wrap } from './wrap'

export type DelayProps = PropsWithChildren<{
ms?: number
Expand All @@ -14,23 +15,13 @@ export const Delay = ({ ms, children }: DelayProps) => {
useTimeout(() => setIsDelayed(true), delayMs)
return <>{isDelayed ? children : null}</>
}
if (process.env.NODE_ENV !== 'production') {
Delay.displayName = 'Delay'
}

export const DelayContext = createContext<PropsWithoutChildren<DelayProps>>({ ms: 0 })

export const withDelay = <TProps extends ComponentProps<ComponentType> = Record<string, never>>(
Component: ComponentType<TProps>,
delayProps?: PropsWithoutChildren<DelayProps>
) => {
const Wrapped = (props: TProps) => (
<Delay {...delayProps}>
<Component {...props} />
</Delay>
)

if (process.env.NODE_ENV !== 'production') {
const name = Component.displayName || Component.name || 'Component'
Wrapped.displayName = `withDelay(${name})`
}

return Wrapped
}
component: ComponentType<TProps>,
delayProps: PropsWithoutChildren<DelayProps> = {}
) => wrap.Delay(delayProps).on(component)
18 changes: 3 additions & 15 deletions packages/react/src/ErrorBoundary.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,7 @@ import {
import { ErrorBoundaryGroupContext } from './ErrorBoundaryGroup'
import type { PropsWithoutChildren } from './types'
import { assert, hasResetKeysChanged } from './utils'
import { wrap } from './wrap'

export type ErrorBoundaryFallbackProps<TError extends Error = Error> = {
/**
Expand Down Expand Up @@ -125,22 +126,9 @@ if (process.env.NODE_ENV !== 'production') {
}

export const withErrorBoundary = <TProps extends ComponentProps<ComponentType> = Record<string, never>>(
Component: ComponentType<TProps>,
component: ComponentType<TProps>,
errorBoundaryProps: PropsWithoutChildren<ErrorBoundaryProps>
) => {
const Wrapped = (props: TProps) => (
<ErrorBoundary {...errorBoundaryProps}>
<Component {...props} />
</ErrorBoundary>
)

if (process.env.NODE_ENV !== 'production') {
const name = Component.displayName || Component.name || 'Component'
Wrapped.displayName = `withErrorBoundary(${name})`
}

return Wrapped
}
) => wrap.ErrorBoundary(errorBoundaryProps).on(component)

const ErrorBoundaryContext = createContext<({ reset: () => void } & ErrorBoundaryState) | null>(null)

Expand Down
23 changes: 7 additions & 16 deletions packages/react/src/ErrorBoundaryGroup.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@ import { createContext, useContext, useEffect, useMemo } from 'react'
import { useIsChanged, useKey } from './hooks'
import type { PropsWithoutChildren } from './types'
import { assert } from './utils'
import { wrap } from './wrap'

export const ErrorBoundaryGroupContext = createContext<{ reset: () => void; resetKey: number } | undefined>(undefined)
if (process.env.NODE_ENV !== 'production') {
Expand Down Expand Up @@ -36,6 +37,9 @@ export const ErrorBoundaryGroup = ({ blockOutside = false, children }: ErrorBoun

return <ErrorBoundaryGroupContext.Provider value={value}>{children}</ErrorBoundaryGroupContext.Provider>
}
if (process.env.NODE_ENV !== 'production') {
ErrorBoundaryGroup.displayName = 'ErrorBoundaryGroup'
}

const ErrorBoundaryGroupReset = ({
trigger: Trigger,
Expand Down Expand Up @@ -67,19 +71,6 @@ export const useErrorBoundaryGroup = () => {
}

export const withErrorBoundaryGroup = <TProps extends ComponentProps<ComponentType> = Record<string, never>>(
Component: ComponentType<TProps>,
errorBoundaryGroupProps?: PropsWithoutChildren<ErrorBoundaryGroupProps>
) => {
const Wrapped = (props: TProps) => (
<ErrorBoundaryGroup {...errorBoundaryGroupProps}>
<Component {...props} />
</ErrorBoundaryGroup>
)

if (process.env.NODE_ENV !== 'production') {
const name = Component.displayName || Component.name || 'Component'
Wrapped.displayName = `withErrorBoundaryGroup(${name})`
}

return Wrapped
}
component: ComponentType<TProps>,
errorBoundaryGroupProps: PropsWithoutChildren<ErrorBoundaryGroupProps> = {}
) => wrap.ErrorBoundaryGroup(errorBoundaryGroupProps).on(component)
43 changes: 9 additions & 34 deletions packages/react/src/Suspense.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@ import type { ComponentProps, ComponentType, ReactNode, SuspenseProps as ReactSu
import { Suspense as ReactSuspense, createContext, useContext } from 'react'
import { useIsClient } from './hooks'
import type { PropsWithoutChildren } from './types'
import { wrap } from './wrap'

export type SuspenseProps = ReactSuspenseProps

Expand Down Expand Up @@ -43,37 +44,11 @@ export const Suspense = DefaultSuspense as typeof DefaultSuspense & {
}
Suspense.CSROnly = CSROnlySuspense

export function withSuspense<TProps extends ComponentProps<ComponentType> = Record<string, never>>(
Component: ComponentType<TProps>,
suspenseProps?: PropsWithoutChildren<SuspenseProps>
) {
const Wrapped = (props: TProps) => (
<Suspense {...suspenseProps}>
<Component {...props} />
</Suspense>
)

if (process.env.NODE_ENV !== 'production') {
const name = Component.displayName || Component.name || 'Component'
Wrapped.displayName = `withSuspense(${name})`
}

return Wrapped
}

withSuspense.CSROnly = function withSuspenseCSROnly<
TProps extends ComponentProps<ComponentType> = Record<string, never>,
>(Component: ComponentType<TProps>, suspenseProps?: PropsWithoutChildren<SuspenseProps>) {
const Wrapped = (props: TProps) => (
<Suspense.CSROnly {...suspenseProps}>
<Component {...props} />
</Suspense.CSROnly>
)

if (process.env.NODE_ENV !== 'production') {
const name = Component.displayName || Component.name || 'Component'
Wrapped.displayName = `withSuspense.CSROnly(${name})`
}

return Wrapped
}
export const withSuspense = <TProps extends ComponentProps<ComponentType> = Record<string, never>>(
component: ComponentType<TProps>,
suspenseProps: PropsWithoutChildren<SuspenseProps> = {}
) => wrap.Suspense(suspenseProps).on(component)
withSuspense.CSROnly = <TProps extends ComponentProps<ComponentType> = Record<string, never>>(
component: ComponentType<TProps>,
suspenseProps: PropsWithoutChildren<SuspenseProps> = {}
) => wrap.Suspense.CSROnly(suspenseProps).on(component)
2 changes: 2 additions & 0 deletions packages/react/src/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,8 @@ export { ErrorBoundaryGroup, withErrorBoundaryGroup, useErrorBoundaryGroup } fro
export { AsyncBoundary, withAsyncBoundary } from './AsyncBoundary'
export { Delay, withDelay } from './Delay'

export { wrap } from './wrap'
Copy link
Member

@minsoo-web minsoo-web Oct 28, 2023

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I want to deprecate all hocs

I agree about this BREAKING CHANGES
but if this function is cover our whole hocs, what about better naming rather than wrap.

My suggestion is with

It is like jest description, using it

it('should error when ....'

it is readable continually by combining it + should

before

// v1
withErrorBoundary(ErrorBoundary, {})(MyComponent)

// v2 wrap case
wrap(ErrorBoundary, {})(MyComponent)

suggestion

with(ErrorBoundary, {})(MyComponent)

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I considered with before too. but in strict mode of JavaScript, with is keyword.
https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Statements/with

And also, because this wrap function is a function of a more abstract layer, it is named wrap, which means it wraps more clearly than with. Just as connect in react-redux has the meaning of connecting a store and a component. with is impossible because it is a keyword, and is there a better naming than wrap?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I see, I don't know about with is keyword in strict mode. wrap is better than I thought another naming. Let's Go !


export type { SuspenseProps } from './Suspense'
export type { ErrorBoundaryProps, ErrorBoundaryFallbackProps } from './ErrorBoundary'
export type { ErrorBoundaryGroupProps } from './ErrorBoundaryGroup'
Expand Down
110 changes: 110 additions & 0 deletions packages/react/src/wrap.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,110 @@
import { createElement } from 'react'
import type { ComponentProps, ComponentType } from 'react'
import type { PropsWithoutChildren } from './types'
import { AsyncBoundary, Delay, ErrorBoundary, ErrorBoundaryGroup, Suspense } from '.'

type WrapperItem<
TWrapperComponent extends
| typeof Suspense
| typeof Suspense.CSROnly
| typeof ErrorBoundary
| typeof ErrorBoundaryGroup
| typeof AsyncBoundary
| typeof AsyncBoundary.CSROnly
| typeof Delay,
> = [TWrapperComponent, PropsWithoutChildren<ComponentProps<TWrapperComponent>>]

type Wrapper =
| WrapperItem<typeof Suspense>
| WrapperItem<typeof Suspense.CSROnly>
| WrapperItem<typeof ErrorBoundary>
| WrapperItem<typeof ErrorBoundaryGroup>
| WrapperItem<typeof AsyncBoundary>
| WrapperItem<typeof AsyncBoundary.CSROnly>
| WrapperItem<typeof Delay>

class WrapWithoutCSROnly {
constructor(private wrappers: Wrapper[]) {}
Suspense = (props: PropsWithoutChildren<ComponentProps<typeof Suspense>> = {}) => {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Do we need to provide default parameter??

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I thought wrap.Suspense, wrap.Suspense.CSROnly, wrap.Delay don't need props object

const Example = wrap.Suspense(/* no parameter is available*/).on(() => {
  return 'success'
})

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

then how about optional type define?

Suggested change
Suspense = (props: PropsWithoutChildren<ComponentProps<typeof Suspense>> = {}) => {
Suspense = (props?: PropsWithoutChildren<ComponentProps<typeof Suspense>>) => {
image

It can prevent unnecessary variable allocation.

Copy link
Member Author

@manudeli manudeli Nov 6, 2023

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Not all wrap.XXX have optional props object. optional props object is only for wrap.Suspense, wrap.Suspense.CSROnly, wrap.Delay. wrap.ErrorBoundary, wrap.ErrorBoundaryGroup require props object.

Making component by wrap will run only one time. so I thought unnecessary variable allocation is okay. but if you have idea to improve performance, could you make Pull Request to this branch? it will make all ci(test, type:check, lint, etc.) and place to easy to be review

this.wrappers.unshift([Suspense, props])
return this
}
ErrorBoundary = (props: PropsWithoutChildren<ComponentProps<typeof ErrorBoundary>>) => {
this.wrappers.unshift([ErrorBoundary, props])
return this
}
ErrorBoundaryGroup = (props: PropsWithoutChildren<ComponentProps<typeof ErrorBoundaryGroup>> = {}) => {
this.wrappers.unshift([ErrorBoundaryGroup, props])
return this
}
AsyncBoundary = (props: PropsWithoutChildren<ComponentProps<typeof AsyncBoundary>>) => {
this.wrappers.unshift([AsyncBoundary, props])
return this
}
Delay = (props: PropsWithoutChildren<ComponentProps<typeof Delay>> = {}) => {
this.wrappers.unshift([Delay, props])
return this
}

on = <TProps extends ComponentProps<ComponentType>>(component: ComponentType<TProps>) => {
const wrappedComponent = (props: TProps) =>
this.wrappers.reduce(
(acc, [wrapperComponent, wrapperProps]) => createElement(wrapperComponent as any, wrapperProps as any, acc),
createElement(component, props)
)

if (process.env.NODE_ENV !== 'production') {
wrappedComponent.displayName = this.wrappers.reduce(
(acc, [wrapperComponent]) => `with${wrapperComponent.displayName}(${acc})`,
component.displayName || component.name || 'Component'
)
}

return wrappedComponent
}
}

type Wrap = WrapWithoutCSROnly & {
Suspense: WrapWithoutCSROnly['Suspense'] & {
CSROnly: (props?: PropsWithoutChildren<ComponentProps<typeof Suspense.CSROnly>>) => Wrap
}
AsyncBoundary: WrapWithoutCSROnly['AsyncBoundary'] & {
CSROnly: (props: PropsWithoutChildren<ComponentProps<typeof AsyncBoundary.CSROnly>>) => Wrap
}
}

const createWrap = () => {
const wrappers: Wrapper[] = []
const builder = new WrapWithoutCSROnly(wrappers) as Wrap
builder.Suspense.CSROnly = (props: PropsWithoutChildren<ComponentProps<typeof Suspense.CSROnly>> = {}) => {
wrappers.unshift([Suspense.CSROnly, props])
return builder
}
builder.AsyncBoundary.CSROnly = (props: PropsWithoutChildren<ComponentProps<typeof AsyncBoundary.CSROnly>>) => {
wrappers.unshift([AsyncBoundary.CSROnly, props])
return builder
}
return builder
}

const wrapSuspense = (...[props = {}]: Parameters<Wrap['Suspense']>) => createWrap().Suspense(props)
wrapSuspense.CSROnly = (...[props = {}]: Parameters<Wrap['Suspense']['CSROnly']>) =>
createWrap().Suspense.CSROnly(props)
const wrapErrorBoundary = (...[props]: Parameters<Wrap['ErrorBoundary']>) => createWrap().ErrorBoundary(props)
const wrapErrorBoundaryGroup = (...[props = {}]: Parameters<Wrap['ErrorBoundaryGroup']>) =>
createWrap().ErrorBoundaryGroup(props)
const wrapAsyncBoundary = (...[props]: Parameters<Wrap['AsyncBoundary']>) => createWrap().AsyncBoundary(props)
wrapAsyncBoundary.CSROnly = (...[props]: Parameters<Wrap['AsyncBoundary']['CSROnly']>) =>
createWrap().AsyncBoundary.CSROnly(props)
const wrapDelay = (...[props = {}]: Parameters<Wrap['Delay']>) => createWrap().Delay(props)

/**
* @experimental This is experimental feature.
*/
export const wrap = {
Suspense: wrapSuspense,
ErrorBoundary: wrapErrorBoundary,
ErrorBoundaryGroup: wrapErrorBoundaryGroup,
AsyncBoundary: wrapAsyncBoundary,
Delay: wrapDelay,
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,24 @@
'use client'

import { useErrorBoundary, wrap } from '@suspensive/react'

const logError = (error: Error) => console.error(error)

const Page = wrap
.ErrorBoundaryGroup({ blockOutside: false })
.ErrorBoundary({ fallback: (props) => <div>{props.error.message}</div>, onError: logError })
.Suspense.CSROnly({ fallback: 'loading...' })
.on(({ text }: { text: string }) => {
const errorBoundary = useErrorBoundary()

return (
<div>
<button onClick={() => errorBoundary.setError(new Error('trigger error by useErrorBoundary().setError'))}>
trigger error by useErrorBoundary().setError
</button>
{text}
</div>
)
})

export default Page
Original file line number Diff line number Diff line change
@@ -0,0 +1,28 @@
'use client'
import { useErrorBoundary, withErrorBoundary, withErrorBoundaryGroup, withSuspense } from '@suspensive/react'
import { UseSuspenseQuery } from '~/components'
import { api } from '~/utils'

const logError = (error: Error) => console.error(error)

export default withErrorBoundaryGroup(
withErrorBoundary(
withSuspense.CSROnly(
() => {
const errorBoundary = useErrorBoundary()

return (
<>
<button onClick={() => errorBoundary.setError(new Error('trigger error by useErrorBoundary().setError'))}>
trigger error by useErrorBoundary().setError
</button>
<UseSuspenseQuery queryKey={['wrap', 1] as const} queryFn={() => api.delay(200, { percentage: 50 })} />
</>
)
},
{ fallback: <>loading...</> }
),
{ fallback: (props) => <>{props.error.message}</>, onError: logError }
),
{ blockOutside: false }
)
Loading
Loading