fix(utils): refactor loadable

[dai-shi]

This is full refactor of the loadable util, introduced in #734. The previous implementation doesn't work with #854.

[vercel]

This pull request is being automatically deployed with Vercel (learn more).
To see the status of your deployment, click below or on the icon next to each commit.

🔍 Inspect: https://vercel.com/pmndrs/jotai/Csjk43uQFDFttPeiuaPAk7fpSESe
✅ Preview: https://jotai-git-fix-utilsrefactor-loadable-pmndrs.vercel.app

[codesandbox-ci]

This pull request is automatically built and testable in CodeSandbox.

To see build info of the built libraries, click here or the icon next to each commit SHA.

Latest deployment of this branch, based on commit 348ad75:

Sandbox	Source
React	Configuration
React Typescript	Configuration
React Browserify	Configuration
React Snowpack	Configuration
Next.js	Configuration
Next.js with custom Babel config	Configuration
React with custom Babel config	Configuration

[dai-shi]

@Pinpickle Would you be able to try this if it doesn't break your use case?

[github-actions]

Size Change: +621 B (+2%)

Total Size: 41.4 kB

Filename	Size	Change
dist/esm/utils.js	4.47 kB	+125 B (+3%)
dist/utils.js	5.72 kB	+496 B (+9%)	🔍

ℹ️ View Unchanged

Filename	Size
dist/babel/plugin-debug-label.js	794 B
dist/babel/plugin-react-refresh.js	947 B
dist/babel/preset.js	1.21 kB
dist/devtools.js	1.73 kB
dist/esm/babel/plugin-debug-label.js	622 B
dist/esm/babel/plugin-react-refresh.js	763 B
dist/esm/babel/preset.js	1.01 kB
dist/esm/devtools.js	1.57 kB
dist/esm/immer.js	597 B
dist/esm/index.js	4.3 kB
dist/esm/optics.js	644 B
dist/esm/query.js	1.25 kB
dist/esm/redux.js	251 B
dist/esm/urql.js	1.44 kB
dist/esm/valtio.js	526 B
dist/esm/xstate.js	1.13 kB
dist/esm/zustand.js	284 B
dist/immer.js	732 B
dist/index.js	5.17 kB
dist/optics.js	938 B
dist/query.js	1.36 kB
dist/redux.js	314 B
dist/urql.js	1.49 kB
dist/valtio.js	586 B
dist/xstate.js	1.19 kB
dist/zustand.js	344 B

_{compressed-size-action}

[Pinpickle]

This looks good to me @dai-shi - if the tests pass then it works for my usecase!

I would suggest adding these tests (which are passing), just to make sure it's not getting tripped up by any async logic in the mix:

it('loadable immediately resolves sync values', async () => {
  const syncAtom = atom(5)
  const effectCallback = jest.fn()

  const { getByText } = render(
    <Provider>
      <LoadableComponent effectCallback={effectCallback} asyncAtom={syncAtom} />
    </Provider>
  )

  getByText('Data: 5')
  expect(effectCallback.mock.calls).not.toContain(
    expect.objectContaining({ state: 'loading' })
  )
  expect(effectCallback).toHaveBeenLastCalledWith({ state: 'hasData', data: 5 })
})

it('loadable can use resolved promises syncronously', async () => {
  const asyncAtom = atom(Promise.resolve(5))
  const effectCallback = jest.fn()

  const ResolveAtomComponent = () => {
    useAtomValue(syncAtom)

    return <div>Ready</div>
  }

  const { getByText, findByText, rerender } = render(
    <Provider>
      <Suspense fallback={null}>
        <ResolveAtomComponent />
      </Suspense>
    </Provider>
  )

  await findByText('Ready')

  rerender(
    <Provider>
      <LoadableComponent effectCallback={effectCallback} asyncAtom={asyncAtom} />
    </Provider>
  )
  getByText('Data: 5')

  expect(effectCallback.mock.calls).not.toContain(
    expect.objectContaining({ state: 'loading' })
  )
  expect(effectCallback).toHaveBeenLastCalledWith({ state: 'hasData', data: 5 })
})

It does require a slight change to the LoadableComponent at the bottom of the file:

interface LoadableComponentProps {
  asyncAtom: Atom<Promise<number> | Promise<string> | string | number>
  effectCallback?: (loadableValue: any) => void
}

const LoadableComponent = ({
  asyncAtom,
  effectCallback,
}: LoadableComponentProps) => {
  const value = useAtomValue(loadable(asyncAtom))

  useEffect(() => {
    if (effectCallback) {
      effectCallback(value)
    }
  }, [value, effectCallback])

  if (value.state === 'loading') {
    return <>Loading...</>
  }

  if (value.state === 'hasError') {
    return <>{String(value.error)}</>
  }

  // this is to ensure correct typing
  const data: number | string = value.data

  return <>Data: {data}</>
}

[dai-shi]

Added the tests. Thanks!