Skip to content

Commit

Permalink
fix(*): add use-sync-external-store to replace useEffect, useState (#596
Browse files Browse the repository at this point in the history
)

fix #566

# Overview

<!--
    A clear and concise description of what this pr is about.
 -->

I add use-sync-external-store correctly

## PR Checklist

- [x] I did below actions if need

1. I read the [Contributing
Guide](https://github.com/suspensive/react/blob/main/CONTRIBUTING.md)
2. I added documents and tests.

---------

Co-authored-by: Eric Butler <[email protected]>
Co-authored-by: Chung-il Jung <[email protected]>
  • Loading branch information
3 people authored Jan 22, 2024
1 parent 8f36141 commit 79948ce
Show file tree
Hide file tree
Showing 31 changed files with 803 additions and 588 deletions.
7 changes: 7 additions & 0 deletions .changeset/yellow-carrots-play.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,7 @@
---
"@suspensive/react-await": patch
"@suspensive/react-image": patch
"@suspensive/react": patch
---

fix(*): add use-sync-external-store
2 changes: 1 addition & 1 deletion .github/workflows/ci.yml
Original file line number Diff line number Diff line change
Expand Up @@ -19,7 +19,7 @@ jobs:
runs-on: ubuntu-latest
strategy:
matrix:
command: ['lint', 'lint:attw', 'lint:monorepo', 'lint:pub', 'test', 'type:check', 'build']
command: ['lint', 'lint:attw', 'lint:monorepo', 'lint:pub', 'test', 'test:production', 'type:check', 'build']
steps:
- uses: actions/checkout@v4
- uses: ./.github/actions/pnpm-setup-node
Expand Down
2 changes: 1 addition & 1 deletion .github/workflows/code-coverage.yml
Original file line number Diff line number Diff line change
Expand Up @@ -22,7 +22,7 @@ jobs:
- uses: ./.github/actions/pnpm-setup-node
- run: pnpm install --frozen-lockfile
- run: pnpm playwright install
- run: pnpm test
- run: pnpm test:production && pnpm test
- name: Upload coverage reports to Codecov
uses: codecov/codecov-action@v3
env:
Expand Down
5 changes: 4 additions & 1 deletion package.json
Original file line number Diff line number Diff line change
Expand Up @@ -27,7 +27,7 @@
"changeset": "changeset",
"changeset:publish": "pnpm prepack && changeset publish",
"changeset:version": "changeset version && pnpm i --lockfile-only",
"ci:all": "pnpm lint && pnpm lint:attw && pnpm lint:monorepo && pnpm lint:pub && pnpm test && pnpm type:check && pnpm build",
"ci:all": "pnpm lint && pnpm lint:attw && pnpm lint:monorepo && pnpm lint:pub && pnpm test && pnpm test:production && pnpm type:check && pnpm build",
"clean": "pnpm --filter \"./packages/**\" run clean",
"commit": "cz",
"dev": "turbo run dev",
Expand All @@ -45,6 +45,8 @@
"prepare": "husky install",
"test": "turbo run test",
"test:watch": "turbo run test:watch --parallel",
"test:production": "turbo run test:production",
"test:production:watch": "turbo run test:production:watch --parallel",
"type:check": "turbo run type:check"
},
"devDependencies": {
Expand All @@ -59,6 +61,7 @@
"@testing-library/dom": "^9.3.4",
"@testing-library/jest-dom": "^6.2.0",
"@testing-library/react": "^14.1.2",
"@testing-library/user-event": "^14.5.2",
"@types/node": "^18.19.8",
"@vitest/browser": "^1.2.1",
"@vitest/coverage-istanbul": "^1.2.1",
Expand Down
2 changes: 1 addition & 1 deletion packages/react-await/src/Await.tsx
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
import { type FunctionComponent, useMemo } from 'react'
import { useSyncExternalStore } from 'use-sync-external-store/shim'
import { useSyncExternalStore } from 'use-sync-external-store/shim/index.js'
import type { Tuple } from './utility-types'
import { hashKey } from './utils'

Expand Down
2 changes: 1 addition & 1 deletion packages/react-image/src/Load.tsx
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
import type { FunctionComponent } from 'react'
import { useSyncExternalStore } from 'use-sync-external-store/shim'
import { useSyncExternalStore } from 'use-sync-external-store/shim/index.js'

/**
* Loads an image from the given source URL.
Expand Down
4 changes: 2 additions & 2 deletions packages/react-query/src/QueryAsyncBoundary.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -18,7 +18,7 @@ const BaseQueryAsyncBoundary = forwardRef<
/>
)
})
if (process.env.NODE_ENV !== 'production') {
if (process.env.NODE_ENV === 'development') {
BaseQueryAsyncBoundary.displayName = 'QueryAsyncBoundary'
}
const CSROnly = forwardRef<
Expand All @@ -37,7 +37,7 @@ const CSROnly = forwardRef<
/>
)
})
if (process.env.NODE_ENV !== 'production') {
if (process.env.NODE_ENV === 'development') {
CSROnly.displayName = 'QueryAsyncBoundary.CSROnly'
}

Expand Down
2 changes: 1 addition & 1 deletion packages/react-query/src/QueryErrorBoundary.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -37,6 +37,6 @@ export const QueryErrorBoundary = forwardRef<
/>
)
})
if (process.env.NODE_ENV !== 'production') {
if (process.env.NODE_ENV === 'development') {
QueryErrorBoundary.displayName = 'QueryErrorBoundary'
}
6 changes: 6 additions & 0 deletions packages/react/package.json
Original file line number Diff line number Diff line change
Expand Up @@ -54,14 +54,20 @@
"prepack": "pnpm build",
"test": "vitest run --coverage --typecheck",
"test:watch": "vitest --ui --coverage --typecheck",
"test:production": "vitest run --config ./vitest-production.config.ts",
"test:production:watch": "vitest --config ./vitest-production.config.ts --ui",
"type:check": "tsc --noEmit"
},
"dependencies": {
"use-sync-external-store": "^1.2.0"
},
"devDependencies": {
"@suspensive/package-json-name": "workspace:*",
"@suspensive/test-utils": "workspace:*",
"@suspensive/tsup": "workspace:*",
"@types/react": "^18.2.48",
"@types/react-dom": "^18.2.18",
"@types/use-sync-external-store": "^0.0.6",
"react": "^18.2.0",
"react-dom": "^18.2.0"
},
Expand Down
4 changes: 2 additions & 2 deletions packages/react/src/AsyncBoundary.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -27,7 +27,7 @@ export const AsyncBoundary = Object.assign(
</ErrorBoundary>
)
)
if (process.env.NODE_ENV !== 'production') {
if (process.env.NODE_ENV === 'development') {
BaseAsyncBoundary.displayName = 'AsyncBoundary'
}

Expand All @@ -47,7 +47,7 @@ export const AsyncBoundary = Object.assign(
</ErrorBoundary>
)
)
if (process.env.NODE_ENV !== 'production') {
if (process.env.NODE_ENV === 'development') {
CSROnly.displayName = 'AsyncBoundary.CSROnly'
}

Expand Down
4 changes: 2 additions & 2 deletions packages/react/src/Delay.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -13,7 +13,7 @@ export interface DelayProps extends PropsWithChildren {
}

export const Delay = (props: DelayProps) => {
if (process.env.NODE_ENV !== 'production') {
if (process.env.NODE_ENV === 'development') {
if (typeof props.ms === 'number') {
assert(props.ms >= 0, DelayMsPropShouldBeGreaterThanOrEqualTo0)
}
Expand All @@ -27,6 +27,6 @@ export const Delay = (props: DelayProps) => {
const fallback = typeof props.fallback === 'undefined' ? defaultProps.fallback : props.fallback
return isDelaying ? fallback : props.children
}
if (process.env.NODE_ENV !== 'production') {
if (process.env.NODE_ENV === 'development') {
Delay.displayName = 'Delay'
}
23 changes: 23 additions & 0 deletions packages/react/src/DevMode.production.spec.tsx
Original file line number Diff line number Diff line change
@@ -0,0 +1,23 @@
import { render, screen } from '@testing-library/react'
import { describe, expect, it } from 'vitest'
import { DevMode } from './DevMode'
import { Suspensive, SuspensiveProvider } from './Suspensive'

describe('<DevMode/> (process.env.NODE_ENV: production)', () => {
it('should show nothing if without SuspensiveProvider', () => {
render(<DevMode />)
expect(screen.queryByRole('Suspensive.DevMode-off')).not.toBeInTheDocument()
expect(screen.queryByRole('Suspensive.DevMode-on')).not.toBeInTheDocument()
})

it('should show nothing on production mode in SuspensiveProvider', () => {
const suspensive = new Suspensive()
const renderResult = render(
<SuspensiveProvider value={suspensive}>
<DevMode />
</SuspensiveProvider>
)

expect(renderResult.queryByRole('Suspensive.DevMode-off')).not.toBeInTheDocument()
})
})
72 changes: 29 additions & 43 deletions packages/react/src/DevMode.spec.tsx
Original file line number Diff line number Diff line change
@@ -1,50 +1,36 @@
import { fireEvent, render } from '@testing-library/react'
import { fireEvent, render, screen } from '@testing-library/react'
import { describe, expect, it } from 'vitest'
import { DevMode } from './DevMode'
import { ErrorBoundary } from './ErrorBoundary'
import { Suspense } from './Suspense'
import { Suspensive, SuspensiveProvider } from './Suspensive'

describe('<DevMode/>', () => {
describe('<DevMode/> (process.env.NODE_ENV: development)', () => {
it('should show nothing if without SuspensiveProvider', () => {
const renderResult = render(<DevMode />)
expect(renderResult.queryByRole('Suspensive.DevMode-off')).not.toBeInTheDocument()
expect(renderResult.queryByRole('Suspensive.DevMode-on')).not.toBeInTheDocument()
render(<DevMode />)
expect(screen.queryByRole('Suspensive.DevMode-off')).not.toBeInTheDocument()
expect(screen.queryByRole('Suspensive.DevMode-on')).not.toBeInTheDocument()
})

it('should show DevMode with role `Suspensive.DevMode: off` in SuspensiveProvider', () => {
const suspensive = new Suspensive()
const renderResult = render(
render(
<SuspensiveProvider value={suspensive}>
<DevMode />
</SuspensiveProvider>
)

expect(renderResult.getByRole('Suspensive.DevMode-off')).toBeInTheDocument()
})

it('should show nothing on production mode in SuspensiveProvider', () => {
process.env.NODE_ENV = 'production'
const suspensive = new Suspensive()
const renderResult = render(
<SuspensiveProvider value={suspensive}>
<DevMode />
</SuspensiveProvider>
)

expect(renderResult.queryByText('Suspensive.DevMode-off')).not.toBeInTheDocument()
process.env.NODE_ENV = undefined
expect(screen.getByRole('Suspensive.DevMode-off')).toBeInTheDocument()
})

it('should scale up itself if hover', () => {
const suspensive = new Suspensive()
const renderResult = render(
render(
<SuspensiveProvider value={suspensive}>
<DevMode />
</SuspensiveProvider>
)

const devMode = renderResult.getByRole('Suspensive.DevMode-off')
const devMode = screen.getByRole('Suspensive.DevMode-off')
expect(devMode).toBeInTheDocument()
expect(devMode.style.transform).toBe('scale(1)')
fireEvent.mouseEnter(devMode)
Expand All @@ -55,21 +41,21 @@ describe('<DevMode/>', () => {

it('should show Suspensive logo with opacity 100% if clicked', () => {
const suspensive = new Suspensive()
const renderResult = render(
render(
<SuspensiveProvider value={suspensive}>
<DevMode />
</SuspensiveProvider>
)

const devMode = renderResult.getByRole('Suspensive.DevMode-off')
const devMode = screen.getByRole('Suspensive.DevMode-off')
expect(devMode).toBeInTheDocument()
fireEvent.click(devMode)
expect(devMode.style.opacity).toBe('1')
})

it('should make Suspense with clientOnly prop show fallback if DevMode is clicked once', () => {
const suspensive = new Suspensive()
const renderResult = render(
render(
<SuspensiveProvider value={suspensive}>
<Suspense
clientOnly
Expand All @@ -84,15 +70,15 @@ describe('<DevMode/>', () => {
</SuspensiveProvider>
)

const devMode = renderResult.getByRole('Suspensive.DevMode-off')
const devMode = screen.getByRole('Suspensive.DevMode-off')
expect(devMode).toBeInTheDocument()
fireEvent.click(devMode)
expect(devMode.style.opacity).toBe('1')
expect(renderResult.getByText('loading...')).toBeInTheDocument()
expect(screen.getByText('loading...')).toBeInTheDocument()
})
it('should make Suspense with clientOnly prop as no devMode if devMode prop is just object', () => {
const suspensive = new Suspensive()
const renderResult = render(
render(
<SuspensiveProvider value={suspensive}>
<Suspense clientOnly fallback="loading..." devMode={{}}>
children
Expand All @@ -101,15 +87,15 @@ describe('<DevMode/>', () => {
</SuspensiveProvider>
)

const devMode = renderResult.getByRole('Suspensive.DevMode-off')
const devMode = screen.getByRole('Suspensive.DevMode-off')
expect(devMode).toBeInTheDocument()
fireEvent.click(devMode)
expect(devMode.style.opacity).toBe('1')
expect(renderResult.getByText('children')).toBeInTheDocument()
expect(screen.getByText('children')).toBeInTheDocument()
})
it('should make Suspense show fallback if DevMode is clicked once', () => {
const suspensive = new Suspensive()
const renderResult = render(
render(
<SuspensiveProvider value={suspensive}>
<Suspense
fallback="loading..."
Expand All @@ -123,15 +109,15 @@ describe('<DevMode/>', () => {
</SuspensiveProvider>
)

const devMode = renderResult.getByRole('Suspensive.DevMode-off')
const devMode = screen.getByRole('Suspensive.DevMode-off')
expect(devMode).toBeInTheDocument()
fireEvent.click(devMode)
expect(devMode.style.opacity).toBe('1')
expect(renderResult.getByText('loading...')).toBeInTheDocument()
expect(screen.getByText('loading...')).toBeInTheDocument()
})
it('should make Suspense as no devMode if devMode prop is just object', () => {
const suspensive = new Suspensive()
const renderResult = render(
render(
<SuspensiveProvider value={suspensive}>
<Suspense fallback="loading..." devMode={{}}>
children
Expand All @@ -140,16 +126,16 @@ describe('<DevMode/>', () => {
</SuspensiveProvider>
)

const devMode = renderResult.getByRole('Suspensive.DevMode-off')
const devMode = screen.getByRole('Suspensive.DevMode-off')
expect(devMode).toBeInTheDocument()
fireEvent.click(devMode)
expect(devMode.style.opacity).toBe('1')
expect(renderResult.getByText('children')).toBeInTheDocument()
expect(screen.getByText('children')).toBeInTheDocument()
})

it('should make ErrorBoundary show fallback if DevMode is clicked once', () => {
const suspensive = new Suspensive()
const renderResult = render(
render(
<SuspensiveProvider value={suspensive}>
<ErrorBoundary
fallback="errorBoundary fallback"
Expand All @@ -163,15 +149,15 @@ describe('<DevMode/>', () => {
</SuspensiveProvider>
)

const devMode = renderResult.getByRole('Suspensive.DevMode-off')
const devMode = screen.getByRole('Suspensive.DevMode-off')
expect(devMode).toBeInTheDocument()
fireEvent.click(devMode)
expect(devMode.style.opacity).toBe('1')
expect(renderResult.getByText('errorBoundary fallback')).toBeInTheDocument()
expect(screen.getByText('errorBoundary fallback')).toBeInTheDocument()
})
it('should make ErrorBoundary as no devMode if devMode prop is just object', () => {
const suspensive = new Suspensive()
const renderResult = render(
render(
<SuspensiveProvider value={suspensive}>
<ErrorBoundary fallback="errorBoundary fallback" devMode={{}}>
children
Expand All @@ -180,10 +166,10 @@ describe('<DevMode/>', () => {
</SuspensiveProvider>
)

const devMode = renderResult.getByRole('Suspensive.DevMode-off')
const devMode = screen.getByRole('Suspensive.DevMode-off')
expect(devMode).toBeInTheDocument()
fireEvent.click(devMode)
expect(devMode.style.opacity).toBe('1')
expect(renderResult.getByText('children')).toBeInTheDocument()
expect(screen.getByText('children')).toBeInTheDocument()
})
})
Loading

0 comments on commit 79948ce

Please sign in to comment.