Skip to content

Commit

Permalink
Merge branch 'main' into v2
Browse files Browse the repository at this point in the history
  • Loading branch information
dai-shi committed Nov 28, 2023
2 parents 2e01d72 + 8109d20 commit 757a064
Show file tree
Hide file tree
Showing 6 changed files with 939 additions and 956 deletions.
1 change: 1 addition & 0 deletions .github/workflows/test-old-typescript.yml
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,7 @@ jobs:
fail-fast: false
matrix:
typescript:
- 5.3.2
- 5.2.2
- 5.1.6
- 5.0.4
Expand Down
40 changes: 20 additions & 20 deletions package.json
Original file line number Diff line number Diff line change
Expand Up @@ -107,49 +107,49 @@
"proxy-compare": "2.5.1"
},
"devDependencies": {
"@babel/core": "^7.23.2",
"@babel/plugin-transform-react-jsx": "^7.22.15",
"@babel/plugin-transform-typescript": "^7.22.15",
"@babel/preset-env": "^7.23.2",
"@redux-devtools/extension": "^3.2.5",
"@rollup/plugin-alias": "^5.0.1",
"@babel/core": "^7.23.3",
"@babel/plugin-transform-react-jsx": "^7.23.4",
"@babel/plugin-transform-typescript": "^7.23.4",
"@babel/preset-env": "^7.23.3",
"@redux-devtools/extension": "^3.2.6",
"@rollup/plugin-alias": "^5.1.0",
"@rollup/plugin-babel": "^6.0.4",
"@rollup/plugin-node-resolve": "^15.2.3",
"@rollup/plugin-replace": "^5.0.5",
"@rollup/plugin-typescript": "^11.1.5",
"@testing-library/react": "^14.0.0",
"@types/jsdom": "^21.1.4",
"@types/react": "^18.2.34",
"@types/react-dom": "^18.2.14",
"@typescript-eslint/eslint-plugin": "^6.9.1",
"@typescript-eslint/parser": "^6.9.1",
"@testing-library/react": "^14.1.2",
"@types/jsdom": "^21.1.6",
"@types/react": "^18.2.39",
"@types/react-dom": "^18.2.17",
"@typescript-eslint/eslint-plugin": "^6.13.0",
"@typescript-eslint/parser": "^6.13.0",
"@vitest/coverage-v8": "0.33.0",
"@vitest/ui": "0.33.0",
"concurrently": "^8.2.2",
"esbuild": "^0.19.5",
"eslint": "^8.52.0",
"esbuild": "^0.19.8",
"eslint": "^8.54.0",
"eslint-config-prettier": "^9.0.0",
"eslint-import-resolver-alias": "^1.1.2",
"eslint-plugin-import": "^2.29.0",
"eslint-plugin-prettier": "^5.0.1",
"eslint-plugin-react": "^7.33.2",
"eslint-plugin-react-hooks": "^4.6.0",
"eslint-plugin-vitest": "^0.3.8",
"jsdom": "^22.1.0",
"eslint-plugin-vitest": "^0.3.10",
"jsdom": "^23.0.0",
"json": "^11.0.0",
"postinstall-postinstall": "^2.1.0",
"prettier": "^3.0.3",
"prettier": "^3.1.0",
"proxy-memoize": "^2.0.4",
"react": "18.3.0-canary-c47c306a7-20231109",
"react-dom": "18.3.0-canary-c47c306a7-20231109",
"redux": "^4.2.1",
"rollup": "^4.2.0",
"rollup": "^4.6.0",
"rollup-plugin-esbuild": "^6.1.0",
"shx": "^0.3.4",
"ts-expect": "^1.3.0",
"tslib": "^2.6.2",
"typescript": "^5.2.2",
"vite": "^4.5.0",
"typescript": "^5.3.2",
"vite": "^5.0.2",
"vitest": "0.33.0"
},
"peerDependencies": {
Expand Down
6 changes: 5 additions & 1 deletion src/react/utils/useProxy.ts
Original file line number Diff line number Diff line change
@@ -1,6 +1,8 @@
import { useLayoutEffect } from 'react'
import { useSnapshot } from '../../react.ts'

const DUMMY_SYMBOL = Symbol()

/**
* useProxy
*
Expand Down Expand Up @@ -28,8 +30,10 @@ export function useProxy<T extends object>(
): T {
const snapshot = useSnapshot(proxy, options) as T

let isRendering = true
// touch dummy prop so that it doesn't trigger re-renders when no props are touched.
;(snapshot as any)[DUMMY_SYMBOL]

let isRendering = true
useLayoutEffect(() => {
// This is an intentional hack
// eslint-disable-next-line react-hooks/exhaustive-deps
Expand Down
39 changes: 24 additions & 15 deletions src/vanilla/utils/watch.ts
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,9 @@ import { subscribe } from '../../vanilla.ts'

type Cleanup = () => void
type WatchGet = <T extends object>(proxyObject: T) => T
type WatchCallback = (get: WatchGet) => Cleanup | void | undefined
type WatchCallback = (
get: WatchGet,
) => Cleanup | void | Promise<Cleanup | void> | undefined
type WatchOptions = {
sync?: boolean
}
Expand Down Expand Up @@ -53,10 +55,12 @@ export function watch(
}
}

const revalidate = () => {
const revalidate = async () => {
if (!alive) {
return
}

// run own cleanups before re-subscribing
cleanups.forEach((clean) => clean())
cleanups.clear()

Expand All @@ -69,35 +73,40 @@ export function watch(

// Ensures that the parent is reset if the callback throws an error.
try {
const cleanupReturn = callback((proxyObject) => {
const promiseOrPossibleCleanup = callback((proxyObject) => {
proxiesToSubscribe.add(proxyObject)
// in case the callback is a promise and the watch has ended
if (alive && !subscriptions.has(proxyObject)) {
// subscribe to new proxy immediately -> this fixes problems when Promises are used due to the callstack
const unsubscribe = subscribe(proxyObject, revalidate, options?.sync)
subscriptions.set(proxyObject, unsubscribe)
}
return proxyObject
})
const couldBeCleanup =
promiseOrPossibleCleanup && promiseOrPossibleCleanup instanceof Promise
? await promiseOrPossibleCleanup
: promiseOrPossibleCleanup

// If there's a cleanup, we add this to the cleanups set
if (cleanupReturn) {
cleanups.add(cleanupReturn)
if (couldBeCleanup) {
if (alive) {
cleanups.add(couldBeCleanup)
} else {
cleanup()
}
}
} finally {
currentCleanups = parent
}

// Unsubscribe old subscriptions
subscriptions.forEach((unsubscribe, proxyObject) => {
if (proxiesToSubscribe.has(proxyObject)) {
// Already subscribed
proxiesToSubscribe.delete(proxyObject)
} else {
if (!proxiesToSubscribe.has(proxyObject)) {
subscriptions.delete(proxyObject)
unsubscribe()
}
})

// Subscribe to new proxies
proxiesToSubscribe.forEach((proxyObject) => {
const unsubscribe = subscribe(proxyObject, revalidate, options?.sync)
subscriptions.set(proxyObject, unsubscribe)
})
}

// If there's a parent watch call, we attach this watch's
Expand Down
67 changes: 66 additions & 1 deletion tests/watch.test.tsx
Original file line number Diff line number Diff line change
@@ -1,8 +1,21 @@
import { describe, expect, it, vi } from 'vitest'
import { afterEach, beforeEach, describe, expect, it, vi } from 'vitest'
import { proxy } from 'valtio'
import { watch } from 'valtio/utils'

const sleep = (ms: number) =>
new Promise((resolve) => {
setTimeout(resolve, ms)
})

describe('watch', () => {
beforeEach(() => {
vi.useFakeTimers()
})

afterEach(() => {
vi.useRealTimers()
})

it('should re-run for individual proxy updates', async () => {
const reference = proxy({ value: 'Example' })

Expand Down Expand Up @@ -96,4 +109,56 @@ describe('watch', () => {

reference.value = 'Update'
})
it('should support promise watchers', async () => {
const reference = proxy({ value: 'Example' })

const callback = vi.fn()

const waitPromise = sleep(10000)
watch(async (get) => {
await waitPromise
get(reference)
callback()
})

vi.runAllTimers()
await waitPromise

expect(callback).toBeCalledTimes(1)
// listener will only be attached after one promise callback due to the await stack
await Promise.resolve()
reference.value = 'Update'
// wait for internal promise
await Promise.resolve()
// wait for next promise resolve call due to promise usage inside of callback
await Promise.resolve()
expect(callback).toBeCalledTimes(2)
})

it('should not subscribe if the watch is stopped before the promise completes', async () => {
const reference = proxy({ value: 'Example' })

const callback = vi.fn()

const waitPromise = sleep(10000)
const stop = watch(async (get) => {
await waitPromise
get(reference)
callback()
})
stop()

vi.runAllTimers()
await waitPromise

expect(callback).toBeCalledTimes(1)
// listener will only be attached after one promise callback due to the await stack
await Promise.resolve()
reference.value = 'Update'
// wait for internal promise
await Promise.resolve()
// wait for next promise resolve call due to promise usage inside of callback
await Promise.resolve()
expect(callback).toBeCalledTimes(1)
})
})
Loading

0 comments on commit 757a064

Please sign in to comment.