diff --git a/CHANGELOG.md b/CHANGELOG.md index ff23a66..3d5b61e 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -1,6 +1,8 @@ # Change Log ## [Unreleased] +### Changed +- fix: no extra calculation with nested memoized selectors #92 ## [2.0.4] - 2023-05-06 ### Changed diff --git a/LICENSE b/LICENSE index 061e77e..8da7df4 100644 --- a/LICENSE +++ b/LICENSE @@ -1,6 +1,6 @@ The MIT License (MIT) -Copyright (c) 2020-2023 Daishi Kato +Copyright (c) 2020 Daishi Kato Permission is hereby granted, free of charge, to any person obtaining a copy of this software and associated documentation files (the "Software"), to deal diff --git a/__tests__/issue_91_spec.ts b/__tests__/issue_91_spec.ts new file mode 100644 index 0000000..6763c88 --- /dev/null +++ b/__tests__/issue_91_spec.ts @@ -0,0 +1,57 @@ +import { memoize } from '../src/index'; + +describe('no extra calculations (#91)', () => { + it('nested memoized selectors with primitive value access', () => { + const reduxState = { + books: { + bookByName: { + bookPrice1: 50, + bookPrice2: 100, + }, + }, + }; + let selectAllBooksCount = 0; + const selectAllBooks = memoize((state: typeof reduxState) => { + selectAllBooksCount += 1; + return Object.values(state.books.bookByName); + }); + const selectSomeBooks = memoize((state: typeof reduxState) => { + const someBooks = selectAllBooks(state); + return someBooks.filter((price) => price === 100); + }); + const selectBooks = memoize((state: typeof reduxState) => { + const allBooks = selectAllBooks(state); + const someBooks = selectSomeBooks(state); + return { allBooks, someBooks }; + }); + selectBooks(reduxState); + expect(selectAllBooksCount).toBe(1); + }); + + it('nested memoized selectors with object access', () => { + const reduxState = { + books: { + bookByName: { + book1: { price: 50 }, + book2: { price: 100 }, + }, + }, + }; + let selectAllBooksCount = 0; + const selectAllBooks = memoize((state: typeof reduxState) => { + selectAllBooksCount += 1; + return Object.values(state.books.bookByName); + }); + const selectSomeBooks = memoize((state: typeof reduxState) => { + const someBooks = selectAllBooks(state); + return someBooks.filter((book) => book.price === 100); + }); + const selectBooks = memoize((state: typeof reduxState) => { + const allBooks = selectAllBooks(state); + const someBooks = selectSomeBooks(state); + return { allBooks, someBooks }; + }); + selectBooks(reduxState); + expect(selectAllBooksCount).toBe(1); + }); +}); diff --git a/package.json b/package.json index 4364573..d71537a 100644 --- a/package.json +++ b/package.json @@ -45,7 +45,7 @@ ], "license": "MIT", "dependencies": { - "proxy-compare": "2.5.1" + "proxy-compare": "2.6.0" }, "devDependencies": { "@types/jest": "^29.5.1", diff --git a/pnpm-lock.yaml b/pnpm-lock.yaml index 966f3d1..5b4cd1c 100644 --- a/pnpm-lock.yaml +++ b/pnpm-lock.yaml @@ -1,9 +1,13 @@ lockfileVersion: '6.0' +settings: + autoInstallPeers: true + excludeLinksFromLockfile: false + dependencies: proxy-compare: - specifier: 2.5.1 - version: 2.5.1 + specifier: 2.6.0 + version: 2.6.0 devDependencies: '@types/jest': @@ -2110,6 +2114,7 @@ packages: /@vue/compiler-core@3.2.47: resolution: {integrity: sha512-p4D7FDnQb7+YJmO2iPEv0SQNeNzcbHdGByJDsT4lynf63AFkOTFN07HsiRSvjGo0QrxR/o3d0hUyNCUnBU2Tig==} + requiresBuild: true dependencies: '@babel/parser': 7.21.8 '@vue/shared': 3.2.47 @@ -2120,6 +2125,7 @@ packages: /@vue/compiler-dom@3.2.47: resolution: {integrity: sha512-dBBnEHEPoftUiS03a4ggEig74J2YBZ2UIeyfpcRM2tavgMWo4bsEfgCGsu+uJIL/vax9S+JztH8NmQerUo7shQ==} + requiresBuild: true dependencies: '@vue/compiler-core': 3.2.47 '@vue/shared': 3.2.47 @@ -2145,6 +2151,7 @@ packages: /@vue/compiler-ssr@3.2.47: resolution: {integrity: sha512-wVXC+gszhulcMD8wpxMsqSOpvDZ6xKXSVWkf50Guf/S+28hTAXPDYRTbLQ3EDkOP5Xz/+SY37YiwDquKbJOgZw==} + requiresBuild: true dependencies: '@vue/compiler-dom': 3.2.47 '@vue/shared': 3.2.47 @@ -2153,6 +2160,7 @@ packages: /@vue/reactivity-transform@3.2.47: resolution: {integrity: sha512-m8lGXw8rdnPVVIdIFhf0LeQ/ixyHkH5plYuS83yop5n7ggVJU+z5v0zecwEnX7fa7HNLBhh2qngJJkxpwEEmYA==} + requiresBuild: true dependencies: '@babel/parser': 7.21.8 '@vue/compiler-core': 3.2.47 @@ -2164,6 +2172,7 @@ packages: /@vue/shared@3.2.47: resolution: {integrity: sha512-BHGyyGN3Q97EZx0taMQ+OLNuZcW3d37ZEVmEAyeoA9ERdGvm9Irc/0Fua8SNyOtV1w6BS4q25wbMzJujO9HIfQ==} + requiresBuild: true dev: true optional: true @@ -3006,6 +3015,7 @@ packages: /de-indent@1.0.2: resolution: {integrity: sha512-e/1zu3xH5MQryN2zdVaF0OrdNLUbvWxzMbi+iNA6Bky7l1RoP8a2fIbRocyHclXt/arDrrR6lL3TqFD9pMQTsg==} + requiresBuild: true dev: true optional: true @@ -4064,6 +4074,7 @@ packages: /he@1.2.0: resolution: {integrity: sha512-F/1DnUGPopORZi0ni+CvrCgHQ5FyEAHRLSApuYWMmrbSwoN2Mn/7k+Gl38gJnR7yyDZk6WLXwiGod1JOWNDKGw==} hasBin: true + requiresBuild: true dev: true optional: true @@ -6450,8 +6461,8 @@ packages: resolution: {integrity: sha512-/XJ368cyBJ7fzLMwLKv1e4vLxOju2MNAIokcr7meSaNcVbWz/CPcW22cP04mwxOErdA5mwjA8Q6w/cdAQxVn7Q==} dev: true - /proxy-compare@2.5.1: - resolution: {integrity: sha512-oyfc0Tx87Cpwva5ZXezSp5V9vht1c7dZBhvuV/y3ctkgMVUmiAGDVeeB0dKhGSyT0v1ZTEQYpe/RXlBVBNuCLA==} + /proxy-compare@2.6.0: + resolution: {integrity: sha512-8xuCeM3l8yqdmbPoYeLbrAXCBWu19XEYc5/F28f5qOaoAIMyfmBUkl5axiK+x9olUvRlcekvnm98AP9RDngOIw==} dev: false /punycode@2.3.0: @@ -6914,6 +6925,7 @@ packages: /sourcemap-codec@1.4.8: resolution: {integrity: sha512-9NykojV5Uih4lgo5So5dtw+f0JgJX30KCNI8gwhz2J9A15wD0Ml6tjHKwf6fTSa6fAdVBdZeNOs9eJ71qCk8vA==} deprecated: Please use @jridgewell/sourcemap-codec instead + requiresBuild: true dev: true /space-separated-tokens@2.0.2: diff --git a/src/memoize.ts b/src/memoize.ts index f83acb6..77b9df2 100644 --- a/src/memoize.ts +++ b/src/memoize.ts @@ -67,6 +67,12 @@ const touchAffected = (dst: unknown, src: unknown, affected: Affected) => { }); }; +const isOriginalEqual = (x: unknown, y: unknown): boolean => { + for (let xx = x; xx; x = xx, xx = getUntracked(xx)); + for (let yy = y; yy; y = yy, yy = getUntracked(yy)); + return Object.is(x, y); +}; + // properties const OBJ_PROPERTY = 'o'; const RESULT_PROPERTY = 'r'; @@ -105,7 +111,13 @@ export function memoize( for (let i = 0; i < size; i += 1) { const memo = memoList[(memoListHead + i) % size]; if (!memo) break; - if (!isChanged(memo[OBJ_PROPERTY], obj, memo[AFFECTED_PROPERTY], new WeakMap())) { + if (!isChanged( + memo[OBJ_PROPERTY], + obj, + memo[AFFECTED_PROPERTY], + new WeakMap(), + isOriginalEqual, + )) { touchAffected(obj, memo[OBJ_PROPERTY], memo[AFFECTED_PROPERTY]); resultCache?.set(obj, memo); return memo[RESULT_PROPERTY];