fix(react): add `<ClientOnly/>` internally #964

Collection50 · 2024-06-24T05:56:15Z

related #528

Overview

Implemented the ClientOnly component.

PR Checklist

I did below actions if need

I read the Contributing Guide
I added documents and tests.

changeset-bot · 2024-06-24T05:56:18Z

🦋 Changeset detected

Latest commit: 4b8d354

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	Patch
@suspensive/react-query	Patch

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 · 2024-06-24T05:56:19Z

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

Name	Status	Preview	Comments	Updated (UTC)
suspensive.org	✅ Ready (Inspect)	Visit Preview	💬 Add feedback	Jun 24, 2024 9:17am
v1.suspensive.org	✅ Ready (Inspect)	Visit Preview	💬 Add feedback	Jun 24, 2024 9:17am
visualization.suspensive.org	✅ Ready (Inspect)	Visit Preview	💬 Add feedback	Jun 24, 2024 9:17am

manudeli · 2024-06-24T07:56:22Z

packages/react/src/components/useIsClient.ts

+import { useSyncExternalStore } from 'react'
+import { noop } from '../utils'
+
+function getSnapshot() {
+  return false
+}
+
+function getServerSnapshot() {
+  return true
+}
+
+// hook interface instead of useIsClient in hooks
+export function useIsClient() {
+  return useSyncExternalStore(() => noop, getSnapshot, getServerSnapshot)
+}


This change is for next stage. Use as-is useIsClient please first for current user.

How about this?

import React, { useState } from 'react' import { noop } from '../utils' import { useIsomorphicLayoutEffect } from './useIsomorphicLayoutEffect' export const useIsClient = () => { const [isClient, setIsClient] = useState(false) useIsomorphicLayoutEffect(() => { setIsClient(true) }, []) if (typeof React['useSyncExternalStore'] === 'function') { return React['useSyncExternalStore']( () => noop, () => true, () => false ) } return isClient }

This is a variant of the existing useIsClient and does not change the interface of the current Suspensive user; rather, the migration to the next stage is much more acceptable.

This is a variant of the existing useIsClient and does not change the interface of the current Suspensive user; rather, the migration to the next stage is much more acceptable.

I agree your opinion and good Idea. but we should change our feature more gradually. I want to check this feature change as beta version. use original useIsClient please. In my opinion, This pull request should be just only for adding ClientOnly component. not improving Suspense clientOnly.

Thank you for your opinion. I'll reflect it.

Thanks for accepting my opinion. I also accept your opinion if we can too. but we should be more careful for this library users. 👍

I'm just grateful to be able to contribute to a great library, with a developer I admire. If there's anything else I can contribute to, I'd love to.

manudeli · 2024-06-24T07:57:03Z

packages/react/src/components/ClientOnly.tsx

+export const ClientOnly = ({ children, fallback }: SuspenseProps) => {
+  const isClient = useIsClient()
+
+  return isClient ? <>{children}</> : <>{fallback}</>
+}


Shortly if we can please

This is ClientOnly. so make type ClientOnlyProps please

Suggested change

export const ClientOnly = ({ children, fallback }: SuspenseProps) => {

const isClient = useIsClient()

return isClient ? <>{children}</> : <>{fallback}</>

}

export const ClientOnly = ({ children, fallback }: ClientOnlyProps) => <>{useIsClient() ? children : fallback}</>

I think most of the time <ClientOnly/> be used as the HOC, so it need a children.
What do you think about using it as?

interface ClienOnlyProp { children: ReactNode: fallback?: ReactNode }

Cool! That's good for me

manudeli · 2024-06-24T07:58:37Z

packages/react/src/Suspense.tsx

+const SuspenseClientOnly = (props: ReactSuspenseProps) => (
+  <ClientOnly fallback={props.fallback}>
+    <ReactSuspense {...props} />
+  </ClientOnly>
+)


manudeli · 2024-06-24T09:06:27Z

packages/react/src/components/ClientOnly.spec.tsx

+import { render, screen } from '@testing-library/react'
+import { useIsClient } from '../hooks/useIsClient'
+import { ClientOnly } from './ClientOnly'
+
+vi.mock('./useIsClient', () => ({
+  useIsClient: vi.fn(),
+}))
+
+afterEach(() => {
+  vi.clearAllMocks()
+})
+
+describe('<ClientOnly/>', () => {
+  it('renders children when isClient is true', () => {
+    useIsClient.mockReturnValue(true)
+
+    render(
+      <ClientOnly fallback={<div>Loading...</div>}>
+        <div>Client Content</div>
+      </ClientOnly>
+    )
+
+    expect(screen.getByText('Client Content')).toBeInTheDocument()
+    expect(screen.queryByText('Loading...')).not.toBeInTheDocument()
+  })
+
+  it('renders fallback when isClient is false', () => {
+    useIsClient.mockReturnValue(false)
+
+    render(
+      <ClientOnly fallback={<div>Loading...</div>}>
+        <div>Client Content</div>
+      </ClientOnly>
+    )
+
+    expect(screen.getByText('Loading...')).toBeInTheDocument()
+    expect(screen.queryByText('Client Content')).not.toBeInTheDocument()
+  })
+})


Cool test code!

manudeli

Thanks! @Collection50

manudeli · 2024-06-24T09:10:26Z

@Collection50 resolve ci:type error please.

codspeed-hq · 2024-06-24T09:21:14Z

CodSpeed Performance Report

Merging #964 will create unknown performance changes

_{Comparing Collection50:ClientOnly (4b8d354) with main (aa9008b)}

Summary

⚠️ No benchmarks were detected in both the base of the PR and the PR.

manudeli

@Collection50 Let's improve clientOnly gradually after this pull request merging

codecov-commenter · 2024-06-24T09:21:28Z

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 80.71%. Comparing base (aa9008b) to head (4b8d354).

Additional details and impacted files

@@            Coverage Diff             @@
##             main     #964      +/-   ##
==========================================
+ Coverage   80.67%   80.71%   +0.04%     
==========================================
  Files          36       37       +1     
  Lines         445      446       +1     
  Branches       98       98              
==========================================
+ Hits          359      360       +1     
  Misses         77       77              
  Partials        9        9

Components	Coverage Δ
@suspensive/react	`96.99% <100.00%> (+0.01%)`	⬆️
@suspensive/react-query	`∅ <ø> (∅)`
@suspensive/react-query-4	`0.00% <ø> (ø)`
@suspensive/react-query-5	`0.00% <ø> (ø)`
@suspensive/react-await	`100.00% <ø> (ø)`
@suspensive/react-image	`23.52% <ø> (ø)`

@Collection50

This PR was opened by the [Changesets release](https://github.com/changesets/action) GitHub action. When you're ready to do a release, you can merge this and the packages will be published to npm automatically. If you're not ready to do a release yet, that's fine, whenever you add more changesets to main, this PR will be updated. # Releases ## @suspensive/react@2.2.1 ### Patch Changes - [#964](#964) [`d87a482`](d87a482) Thanks [@Collection50](https://github.com/Collection50)! - fix(react): add `<ClienOnly/>` for clientOnly prop of `<Suspense/>` ## @suspensive/react-query@2.2.1 ### Patch Changes - Updated dependencies \[[`d87a482`](d87a482)]: - @suspensive/react@2.2.1 - @suspensive/react-query-4@0.0.1 - @suspensive/react-query-5@0.0.1 Co-authored-by: github-actions[bot] <github-actions[bot]@users.noreply.github.com>

related #528 # Overview Implemented the `ClientOnly` component. ## PR Checklist - [x] I did below actions if need 1. I read the [Contributing Guide](https://github.com/toss/suspensive/blob/main/CONTRIBUTING.md) 2. I added documents and tests. --------- Co-authored-by: Jonghyeon Ko <jonghyeon@toss.im>

@Collection50

This PR was opened by the [Changesets release](https://github.com/changesets/action) GitHub action. When you're ready to do a release, you can merge this and the packages will be published to npm automatically. If you're not ready to do a release yet, that's fine, whenever you add more changesets to main, this PR will be updated. # Releases ## @suspensive/react@2.2.1 ### Patch Changes - [#964](#964) [`d87a482`](d87a482) Thanks [@Collection50](https://github.com/Collection50)! - fix(react): add `<ClienOnly/>` for clientOnly prop of `<Suspense/>` ## @suspensive/react-query@2.2.1 ### Patch Changes - Updated dependencies \[[`d87a482`](d87a482)]: - @suspensive/react@2.2.1 - @suspensive/react-query-4@0.0.1 - @suspensive/react-query-5@0.0.1 Co-authored-by: github-actions[bot] <github-actions[bot]@users.noreply.github.com>

related #528 # Overview Implemented the `ClientOnly` component. ## PR Checklist - [x] I did below actions if need 1. I read the [Contributing Guide](https://github.com/toss/suspensive/blob/main/CONTRIBUTING.md) 2. I added documents and tests. ---------

@Collection50

This PR was opened by the [Changesets release](https://github.com/changesets/action) GitHub action. When you're ready to do a release, you can merge this and the packages will be published to npm automatically. If you're not ready to do a release yet, that's fine, whenever you add more changesets to main, this PR will be updated. # Releases ## @suspensive/react@2.2.1 ### Patch Changes - [#964](#964) [`d87a482`](d87a482) Thanks [@Collection50](https://github.com/Collection50)! - fix(react): add `<ClienOnly/>` for clientOnly prop of `<Suspense/>` ## @suspensive/react-query@2.2.1 ### Patch Changes - Updated dependencies \[[`d87a482`](d87a482)]: - @suspensive/react@2.2.1 - @suspensive/react-query-4@0.0.1 - @suspensive/react-query-5@0.0.1

related #528 # Overview Implemented the `ClientOnly` component. ## PR Checklist - [x] I did below actions if need 1. I read the [Contributing Guide](https://github.com/toss/suspensive/blob/main/CONTRIBUTING.md) 2. I added documents and tests. --------- Co-authored-by: Jonghyeon Ko <jonghyeon@toss.im>

@Collection50

This PR was opened by the [Changesets release](https://github.com/changesets/action) GitHub action. When you're ready to do a release, you can merge this and the packages will be published to npm automatically. If you're not ready to do a release yet, that's fine, whenever you add more changesets to main, this PR will be updated. # Releases ## @suspensive/react@2.2.1 ### Patch Changes - [#964](#964) [`d87a482`](d87a482) Thanks [@Collection50](https://github.com/Collection50)! - fix(react): add `<ClienOnly/>` for clientOnly prop of `<Suspense/>` ## @suspensive/react-query@2.2.1 ### Patch Changes - Updated dependencies \[[`d87a482`](d87a482)]: - @suspensive/react@2.2.1 - @suspensive/react-query-4@0.0.1 - @suspensive/react-query-5@0.0.1

related #528 # Overview Implemented the `ClientOnly` component. ## PR Checklist - [x] I did below actions if need 1. I read the [Contributing Guide](https://github.com/toss/suspensive/blob/main/CONTRIBUTING.md) 2. I added documents and tests. --------- Co-authored-by: Jonghyeon Ko <jonghyeon@toss.im>

@Collection50

This PR was opened by the [Changesets release](https://github.com/changesets/action) GitHub action. When you're ready to do a release, you can merge this and the packages will be published to npm automatically. If you're not ready to do a release yet, that's fine, whenever you add more changesets to main, this PR will be updated. # Releases ## @suspensive/react@2.2.1 ### Patch Changes - [#964](#964) [`d87a482`](d87a482) Thanks [@Collection50](https://github.com/Collection50)! - fix(react): add `<ClienOnly/>` for clientOnly prop of `<Suspense/>` ## @suspensive/react-query@2.2.1 ### Patch Changes - Updated dependencies \[[`d87a482`](d87a482)]: - @suspensive/react@2.2.1 - @suspensive/react-query-4@0.0.1 - @suspensive/react-query-5@0.0.1

Collection50 added 5 commits June 24, 2024 14:06

feat: implement component ClientOnly, and useIsClient hook

8233f4f

docs: add typo

e9ed50b

test: tescode for ClientOnly

25bf926

docs: fix typo

0414ee5

test: fix typo in test code

d003781

Collection50 requested review from manudeli, okinawaa and minsoo-web as code owners June 24, 2024 05:56

github-actions bot added the @suspensive/react label Jun 24, 2024

vercel bot deployed to Preview – suspensive.org June 24, 2024 05:56 View deployment

vercel bot deployed to Preview – v1.suspensive.org June 24, 2024 05:57 View deployment

vercel bot had a problem deploying to Preview – visualization.suspensive.org June 24, 2024 05:58 Failure

manudeli assigned Collection50 Jun 24, 2024

manudeli requested changes Jun 24, 2024

View reviewed changes

manudeli changed the title ~~feat(react): add ClienOnly component~~ fix(react): add ClientOnly component internally Jun 24, 2024

Create thirty-panthers-flash.md

337b875

manudeli changed the title ~~fix(react): add ClientOnly component internally~~ fix(react): add <ClientOnly/> internally Jun 24, 2024

Merge branch 'main' into ClientOnly

68a04a7

vercel bot deployed to Preview – v1.suspensive.org June 24, 2024 08:49 View deployment

vercel bot deployed to Preview – suspensive.org June 24, 2024 08:50 View deployment

vercel bot had a problem deploying to Preview – visualization.suspensive.org June 24, 2024 08:51 Failure

Collection50 added 2 commits June 24, 2024 17:51

fix: add ClientOnly interface

4352b03

fix: fix ClientOnly logic

76a5003

vercel bot deployed to Preview – v1.suspensive.org June 24, 2024 08:51 View deployment

vercel bot deployed to Preview – suspensive.org June 24, 2024 08:52 View deployment

vercel bot deployed to Preview – visualization.suspensive.org June 24, 2024 08:53 View deployment

manudeli reviewed Jun 24, 2024

View reviewed changes

manudeli approved these changes Jun 24, 2024

View reviewed changes

fix: type error

f63a253

vercel bot deployed to Preview – suspensive.org June 24, 2024 09:13 View deployment

vercel bot deployed to Preview – v1.suspensive.org June 24, 2024 09:13 View deployment

test: fix import path of useIsClient

4b8d354

vercel bot deployed to Preview – suspensive.org June 24, 2024 09:15 View deployment

vercel bot deployed to Preview – visualization.suspensive.org June 24, 2024 09:17 View deployment

vercel bot deployed to Preview – v1.suspensive.org June 24, 2024 09:17 View deployment

manudeli approved these changes Jun 24, 2024

View reviewed changes

manudeli merged commit d87a482 into toss:main Jun 24, 2024
14 checks passed

github-actions bot mentioned this pull request Jun 24, 2024

chore: version packages #968

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

fix(react): add `<ClientOnly/>` internally #964

fix(react): add `<ClientOnly/>` internally #964

Collection50 commented Jun 24, 2024 •

edited by manudeli

Loading

changeset-bot bot commented Jun 24, 2024 •

edited

Loading

vercel bot commented Jun 24, 2024 •

edited

Loading

manudeli Jun 24, 2024

Collection50 Jun 24, 2024

Collection50 Jun 24, 2024

manudeli Jun 24, 2024 •

edited

Loading

Collection50 Jun 24, 2024

manudeli Jun 24, 2024

Collection50 Jun 24, 2024

manudeli Jun 24, 2024

Collection50 Jun 24, 2024

manudeli Jun 24, 2024

manudeli Jun 24, 2024

manudeli Jun 24, 2024

manudeli left a comment

manudeli commented Jun 24, 2024

codspeed-hq bot commented Jun 24, 2024

manudeli left a comment

codecov-commenter commented Jun 24, 2024

fix(react): add <ClientOnly/> internally #964

fix(react): add <ClientOnly/> internally #964

Conversation

Collection50 commented Jun 24, 2024 • edited by manudeli Loading

Overview

PR Checklist

changeset-bot bot commented Jun 24, 2024 • edited Loading

🦋 Changeset detected

vercel bot commented Jun 24, 2024 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

manudeli Jun 24, 2024 • 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

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

Choose a reason for hiding this comment

manudeli left a comment

Choose a reason for hiding this comment

manudeli commented Jun 24, 2024

codspeed-hq bot commented Jun 24, 2024

Merging #964 will create unknown performance changes

Summary

manudeli left a comment

Choose a reason for hiding this comment

codecov-commenter commented Jun 24, 2024

Codecov Report

fix(react): add `<ClientOnly/>` internally #964

fix(react): add `<ClientOnly/>` internally #964

Collection50 commented Jun 24, 2024 •

edited by manudeli

Loading

changeset-bot bot commented Jun 24, 2024 •

edited

Loading

vercel bot commented Jun 24, 2024 •

edited

Loading

manudeli Jun 24, 2024 •

edited

Loading