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

manudeli · 2023-10-28T09:48:41Z

Overview

I added wrap as experimental feature to remove all repetition of hocs implementation and withXXX funciton's nesting.
Library size has been reduced by 12%(2,114 bytes) than before

AS-IS dist/index.cjs

18,377 bytes

TO-BE dist/index.cjs if add wrap and remove hocs

17,120 bytes

I want to deprecate all hocs

In v1, I hope to add the wrap function as a public API and keep all hocs marked as deprecated, and in v2, I hope to remove all hocs and provide only the wrap function.

Why want to deprecate all hocs

To reduce library size more
User don't have to know withSuspense, withSuspense.CSROnly, withErrorBoudnary, withAsyncBoundary, withAsyncBoundary.CSROnly, withErrorBoundaryGroup, withDelay. just have to know wrap and existing components. (number of public apis, 15(with hocs) -> 11(without hocs))

wrap function allows us to place related code in a more relevant location. We can also create a function wrapped with wrap to reuse it because calling wrap will make hoc.

withXXX

export default withErrorBoundary(
  withSuspense(
    () => {
      return 'success'
    },
    { fallback: 'loading...' } // far position with name of withSuspense
  ),
  { fallback: () => 'error' } // far position with name of withErrorBoundary
)

wrap

export default wrap
  .ErrorBoundary({ fallback: () => 'error' })
  .Suspense({ fallback: 'loading...' })
  .on(() => {
    return 'success'
  })

PR Checklist

I did below actions if need

I read the Contributing Guide
I added documents and tests.

changeset-bot · 2023-10-28T09:48:45Z

🦋 Changeset detected

Latest commit: 20db7a3

The changes in this PR will be included in the next version bump.

This PR includes changesets to release 2 packages

Name	Type
@suspensive/react	Minor
@suspensive/react-query	Minor

Not sure what this means? Click here to learn what changesets are.

Click here if you're a maintainer who wants to add another changeset to this PR

vercel · 2023-10-28T09:48:48Z

The latest updates on your projects. Learn more about Vercel for Git ↗︎

Name	Status	Preview	Comments	Updated (UTC)
docs	✅ Ready (Inspect)	Visit Preview	💬 Add feedback	Nov 9, 2023 3:29pm
visualization	✅ Ready (Inspect)	Visit Preview	💬 Add feedback	Nov 9, 2023 3:29pm

codecov · 2023-10-28T09:51:08Z

Codecov Report

Merging #270 (20db7a3) into main (498398e) will not change coverage.
The diff coverage is 100.00%.

@@            Coverage Diff            @@
##              main      #270   +/-   ##
=========================================
  Coverage   100.00%   100.00%           
=========================================
  Files           23        24    +1     
  Lines          806       838   +32     
  Branches       136       143    +7     
=========================================
+ Hits           806       838   +32

Components	Coverage Δ
@suspensive/react	`100.00% <100.00%> (ø)`
@suspensive/react-query	`∅ <ø> (∅)`
@suspensive/react-await	`100.00% <ø> (ø)`

manudeli · 2023-10-28T10:26:18Z

packages/react/src/AsyncBoundary.tsx

-) => {
-  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
-}


Unnecessary implementations of all hocs is removed by wrap function

manudeli · 2023-10-28T10:27:48Z

websites/visualization/src/app/react/experimental/wrap/after/page.tsx

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


We can compare before, after of wrap function
between websites/visualization/src/app/react/experimental/wrap/after/page.tsx and websites/visualization/src/app/react/experimental/wrap/before/page.tsx

Check it please

Can I compare it through the network tab? Since the user doesn't write the wrap function himself, do I just need to look at the performance aspect?

Can I compare it through the network tab? Since the user doesn't write the wrap function himself, do I just need to look at the performance aspect in network tab?

Sure, Could you please suggest how can we compare performance?

however, this change can greatly reduce the file size of the library without BREAKING CHANGE. This is a definite improvement. Additionally, the number of public APIs users have to know can be greatly reduced. I think this is a more important improvement than performance. So I hope that the performance comparison does not delay the merge of this feature. Before merging, it would be better to talk about the interface of wrap (tentative name) rather than comparing performance.

Oh I see, I agree about your opinion

.changeset/pretty-pumpkins-jam.md

minsoo-web · 2023-10-28T16:11:39Z

packages/react/src/index.ts

@@ -5,6 +5,8 @@ export { ErrorBoundaryGroup, withErrorBoundaryGroup, useErrorBoundaryGroup } fro
 export { AsyncBoundary, withAsyncBoundary } from './AsyncBoundary'
 export { Delay, withDelay } from './Delay'

+export { wrap } from './wrap'


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)

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?

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

minsoo-web · 2023-10-28T16:23:07Z

websites/visualization/src/app/react/experimental/wrap/after/page.tsx

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


Can I compare it through the network tab? Since the user doesn't write the wrap function himself, do I just need to look at the performance aspect?

minsoo-web

I think you used default parameters often, is there a reason?

minsoo-web · 2023-11-06T02:44:51Z

packages/react/src/wrap.ts

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


Do we need to provide default parameter??

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' })

then how about optional type define?

Suggested change

Suspense = (props: PropsWithoutChildren<ComponentProps<typeof Suspense>> = {}) => {

Suspense = (props?: PropsWithoutChildren<ComponentProps<typeof Suspense>>) => {

It can prevent unnecessary variable allocation.

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

manudeli · 2023-11-09T16:49:18Z

@okinawaa wrap is experimental feature. I merge this to make new release contain this feature quickly. this will make us get feedback to improve this experimental feature easily and we can fix it later. Feedback this please later

fix #269 # Overview  I added `wrap` as experimental feature to remove all repetition of hocs implementation and withXXX funciton's nesting. Library size has been reduced by 12%(2,114 bytes) than before ### AS-IS dist/index.cjs 18,377 bytes <img width="261" alt="image" src="https://github.com/suspensive/react/assets/61593290/c891fd26-2000-4fd9-8a5f-5712c0adce90"> ### TO-BE dist/index.cjs if add wrap and remove hocs 17,120 bytes <img width="261" alt="image" src="https://github.com/suspensive/react/assets/61593290/8df4582f-2420-495c-8649-0adf65651abd"> ## I want to deprecate all hocs In v1, I hope to add the `wrap` function as a public API and keep all hocs marked as deprecated, and in v2, I hope to remove all hocs and provide only the `wrap` function. ### Why want to deprecate all hocs 1. To reduce library size more 2. User don't have to know withSuspense, withSuspense.CSROnly, withErrorBoudnary, withAsyncBoundary, withAsyncBoundary.CSROnly, withErrorBoundaryGroup, withDelay. just have to know wrap and existing components. (number of public apis, 15(with hocs) -> 11(without hocs)) 3. `wrap` function allows us to place related code in a more relevant location. We can also create a function wrapped with wrap to reuse it because calling `wrap` will make hoc. #### withXXX ```tsx export default withErrorBoundary( withSuspense( () => { return 'success' }, { fallback: 'loading...' } // far position with name of withSuspense ), { fallback: () => 'error' } // far position with name of withErrorBoundary ) ``` #### wrap ```tsx export default wrap .ErrorBoundary({ fallback: () => 'error' }) .Suspense({ fallback: 'loading...' }) .on(() => { return 'success' }) ``` ## PR Checklist - [x] I did below actions if need 1. I read the [Contributing Guide](https://github.com/suspensive/react/blob/main/CONTRIBUTING.md) 5. I added documents and tests.

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

e2067db

manudeli self-assigned this Oct 28, 2023

github-actions bot added visualization @suspensive/react labels Oct 28, 2023

vercel bot temporarily deployed to Preview – visualization October 28, 2023 09:49 Inactive

vercel bot temporarily deployed to Preview – docs October 28, 2023 09:49 Inactive

chore: update

5e85da0

vercel bot temporarily deployed to Preview – docs October 28, 2023 09:52 Inactive

vercel bot temporarily deployed to Preview – visualization October 28, 2023 09:53 Inactive

Create pretty-pumpkins-jam.md

b93b7a8

vercel bot temporarily deployed to Preview – docs October 28, 2023 10:24 Inactive

manudeli commented Oct 28, 2023

View reviewed changes

Update .changeset/pretty-pumpkins-jam.md

527164f

manudeli marked this pull request as ready for review October 28, 2023 10:28

manudeli requested review from okinawaa and minsoo-web as code owners October 28, 2023 10:28

vercel bot temporarily deployed to Preview – docs October 28, 2023 10:28 Inactive

minsoo-web reviewed Oct 28, 2023

View reviewed changes

Merge branch 'main' into react/feat/wrap

4d38677

vercel bot temporarily deployed to Preview – visualization October 29, 2023 09:45 Inactive

vercel bot temporarily deployed to Preview – docs October 29, 2023 09:46 Inactive

Merge branch 'main' into react/feat/wrap

de308f8

vercel bot temporarily deployed to Preview – docs October 29, 2023 09:53 Inactive

vercel bot temporarily deployed to Preview – visualization October 29, 2023 09:54 Inactive

Merge branch 'main' into react/feat/wrap

b1f5f04

vercel bot temporarily deployed to Preview – docs October 29, 2023 10:05 Inactive

vercel bot temporarily deployed to Preview – visualization October 29, 2023 10:06 Inactive

vercel bot temporarily deployed to Preview – visualization November 5, 2023 03:06 Inactive

chore: resolve lint

cae241a

github-actions bot removed the @suspensive/react-query label Nov 5, 2023

vercel bot temporarily deployed to Preview – docs November 5, 2023 03:09 Inactive

vercel bot temporarily deployed to Preview – visualization November 5, 2023 03:10 Inactive

Merge branch 'main' into react/feat/wrap

95cb71e

vercel bot temporarily deployed to Preview – visualization November 5, 2023 03:19 Inactive

vercel bot temporarily deployed to Preview – docs November 5, 2023 03:20 Inactive

minsoo-web reviewed Nov 6, 2023

View reviewed changes

Merge branch 'main' into react/feat/wrap

8273979

vercel bot temporarily deployed to Preview – visualization November 6, 2023 04:14 Inactive

vercel bot temporarily deployed to Preview – docs November 6, 2023 04:15 Inactive

Merge branch 'main' into react/feat/wrap

13d856e

vercel bot temporarily deployed to Preview – docs November 6, 2023 15:46 Inactive

vercel bot temporarily deployed to Preview – visualization November 6, 2023 15:47 Inactive

Merge branch 'main' into react/feat/wrap

b92b159

vercel bot temporarily deployed to Preview – docs November 7, 2023 16:42 Inactive

vercel bot temporarily deployed to Preview – visualization November 7, 2023 16:43 Inactive

Merge branch 'main' into react/feat/wrap

75238e8

vercel bot temporarily deployed to Preview – docs November 8, 2023 09:52 Inactive

vercel bot temporarily deployed to Preview – visualization November 8, 2023 09:54 Inactive

Merge branch 'main' into react/feat/wrap

20db7a3

vercel bot temporarily deployed to Preview – docs November 9, 2023 15:27 Inactive

vercel bot temporarily deployed to Preview – visualization November 9, 2023 15:29 Inactive

manudeli merged commit f78ed7d into main Nov 9, 2023
19 checks passed

manudeli deleted the react/feat/wrap branch November 9, 2023 16:50

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

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

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

manudeli commented Oct 28, 2023 •

edited

Loading

changeset-bot bot commented Oct 28, 2023 •

edited

Loading

vercel bot commented Oct 28, 2023 •

edited

Loading

codecov bot commented Oct 28, 2023 •

edited

Loading

manudeli Oct 28, 2023

manudeli Oct 28, 2023

minsoo-web Oct 28, 2023

manudeli Oct 28, 2023 •

edited

Loading

minsoo-web Oct 29, 2023

minsoo-web Oct 28, 2023 •

edited

Loading

manudeli Oct 28, 2023

minsoo-web Oct 29, 2023

minsoo-web Oct 28, 2023

minsoo-web left a comment •

edited

Loading

minsoo-web Nov 6, 2023

manudeli Nov 6, 2023

minsoo-web Nov 6, 2023

manudeli Nov 6, 2023 •

edited

Loading

manudeli commented Nov 9, 2023 •

edited

Loading

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

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

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

Conversation

manudeli commented Oct 28, 2023 • edited Loading

Overview

AS-IS dist/index.cjs

TO-BE dist/index.cjs if add wrap and remove hocs

I want to deprecate all hocs

Why want to deprecate all hocs

withXXX

wrap

PR Checklist

changeset-bot bot commented Oct 28, 2023 • edited Loading

🦋 Changeset detected

vercel bot commented Oct 28, 2023 • edited Loading

codecov bot commented Oct 28, 2023 • edited Loading

Codecov Report

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

manudeli Oct 28, 2023 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

minsoo-web Oct 28, 2023 • edited Loading

Choose a reason for hiding this comment

before

suggestion

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

minsoo-web left a comment • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

manudeli Nov 6, 2023 • edited Loading

Choose a reason for hiding this comment

manudeli commented Nov 9, 2023 • edited Loading

manudeli commented Oct 28, 2023 •

edited

Loading

changeset-bot bot commented Oct 28, 2023 •

edited

Loading

vercel bot commented Oct 28, 2023 •

edited

Loading

codecov bot commented Oct 28, 2023 •

edited

Loading

manudeli Oct 28, 2023 •

edited

Loading

minsoo-web Oct 28, 2023 •

edited

Loading

minsoo-web left a comment •

edited

Loading

manudeli Nov 6, 2023 •

edited

Loading

manudeli commented Nov 9, 2023 •

edited

Loading