Skip to content

Commit

Permalink
fix(core): handle edge cases, not to mount dependency atoms if the at…
Browse files Browse the repository at this point in the history
…om is not mounted (#961)

* fix: add test case for #942

* fix to pass the test

Co-authored-by: daishi <daishi@axlight.com>
  • Loading branch information
xbaun and dai-shi authored Jan 18, 2022
1 parent 3610734 commit dc6851d
Show file tree
Hide file tree
Showing 2 changed files with 33 additions and 1 deletion.
5 changes: 4 additions & 1 deletion src/core/store.ts
Original file line number Diff line number Diff line change
Expand Up @@ -804,7 +804,10 @@ export const createStore = (
const mounted = mountedMap.get(a)
if (mounted) {
mounted.t.add(atom) // add to dependents
} else {
} else if (mountedMap.has(atom)) {
// we mount dependencies only when atom is already mounted
// Note: we should revisit this when you find other issues
// https://github.com/pmndrs/jotai/issues/942
mountAtom(a, atom)
}
})
Expand Down
29 changes: 29 additions & 0 deletions tests/basic.test.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -987,3 +987,32 @@ it('chained derive atom with onMount and useEffect (#897)', async () => {

await findByText('count: 1')
})

it('onMount is not called when atom value is accessed from writeGetter in derived atom (#942)', async () => {
const onUnmount = jest.fn()
const onMount = jest.fn(() => {
return onUnmount
})

const aAtom = atom(false)
aAtom.onMount = onMount

const bAtom = atom(null, (get) => {
get(aAtom)
})

const App = () => {
const [, action] = useAtom(bAtom)
useEffect(() => action(), [action])
return null
}

render(
<Provider>
<App />
</Provider>
)

expect(onMount).not.toBeCalled()
expect(onUnmount).not.toBeCalled()
})

1 comment on commit dc6851d

@vercel
Copy link

@vercel vercel bot commented on dc6851d Jan 18, 2022

Choose a reason for hiding this comment

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

Please sign in to comment.