Static props prevents selectors recall

[Bardiamist]

Hey, I have reproducible problem on big project but I still can't create small example.

Essence of the problem:
proxy-memoize detects changes in the selector and recall as expected.
But if this selector will additionally use some string like state.books[id].priceString (where priceString is '0') then proxy-memoize doesn't recall this selector.

Additional info:
Selector uses another memoized selector. If don't memoize internal selector then no problems with '0' strings.

Any ideas?

[dai-shi]

Thanks for reporting.
Does it only fail with '0' and not with '1'?

We would need a test case to fix the bug.

[Bardiamist]

Looks it fails with any string when this string not changed between calls.

[Bardiamist]

Not only strings. Also numbers and {} leads to this problem.
I tied to create small repro many hours, copied many selectors. But it reproducing only on the project.
Maybe will continue try tomorow.
Any ideas would be helpful.

[dai-shi]

Selector uses another memoized selector.

We have some double selector tests, but the coverage may not be enough.
For triple selector tests, test coverage would be lower.

I'm not sure if that helps.

[Bardiamist]

Reproduced

const memoize = require('proxy-memoize').memoize;

const state1 = {
  book1: {
    staticProp: '5',
    priceString: '10',
  },
};

const state2 = {
  book1: {
    staticProp: '5',
    priceString: '20',
  },
};

const selectAllBooks = memoize((state) => Object.values(state));

const selectPriceString = memoize((
  state,
) => state.book1.priceString);

const selectAdjustedPriceString = memoize((state) => {
  const priceString = selectPriceString(state);
  
  const problem = state.book1.staticProp;

  return priceString;
});

selectAllBooks(state1);
console.log(selectAdjustedPriceString(state1));
console.log(selectAdjustedPriceString(state2));

node version: 22.4.0
proxy-memoize version: 3.0.0

Second console.log should show 20 (not 10)

[dai-shi]

Thanks for the reproduction. #101 adds the test case.

[dai-shi]

#101 should fix.

Please try:
https://ci.codesandbox.io/status/dai-shi/proxy-memoize/pr/101
☝️ Find "Local Install Instructions"

[Bardiamist]

I use yarn 4 so used yarn add proxy-memoize@https://pkg.csb.dev/dai-shi/proxy-memoize/commit/4aac6ae2/proxy-memoize/_pkg.tgz

This case fixed, thanks 🎉

But my initial problem still exists 😢 I'll back if will find way to create a repro.

[Bardiamist]

You can start from this

Simplified example

const memoize = require('proxy-memoize').memoize;

const state1 = {
  booksState: {
    booksObject: {
      'book1-name': {
        shortName: 'book1-name',
      },
    },
  },
  extendedBooksState: {
    extendedBooksObject: {
      'book1-name': {
        staticProp: '5',
        priceString: '10',
        book: {
          shortName: 'book1-name',
        },
      },
    },
  },
};

const state2 = {
  booksState: {
    booksObject: {
      'book1-name': {
        shortName: 'book1-name',
      },
    },
  },
  extendedBooksState: {
    extendedBooksObject: {
      'book1-name': {
        staticProp: '5',
        priceString: '0',
        book: {
          shortName: 'book1-name',
        },
      },
    },
  },
};

const selectPriceString = memoize((
  state,
) => state.extendedBooksState.extendedBooksObject['book1-name'].priceString);

const selectAdjustedPriceString = memoize((state) => {
  const {
    booksState: {
      booksObject,
    },
    extendedBooksState: {
      extendedBooksObject,
    },
  } = state;

  const book = booksObject['book1-name'];

  book.shortName; // touch the prop

  const priceString = selectPriceString(state);

  extendedBooksObject['book1-name'].staticProp; // touch the prop

  return priceString;
});

const selectAllExtendedBooks = memoize((
  state,
) => Object.values(state.extendedBooksState.extendedBooksObject));

const selectBookThroughExtendedBooks = memoize((state) => selectAllExtendedBooks(state)[0].book);

const selectMemoizedPriceString = memoize((state) => selectPriceString(state).priceString);

const getPriceStringSelector = (
  book // looks as touch
) => (
  state,
) => selectMemoizedPriceString(state);

getPriceStringSelector(selectBookThroughExtendedBooks(state1))(state1);
console.log(selectAdjustedPriceString(state1));

getPriceStringSelector(selectBookThroughExtendedBooks(state2))(state2);
console.log(selectAdjustedPriceString(state2));

Second expected output is 0

[Bardiamist]

Here is extended example which more close to my problem

Extended example

const memoize = require('proxy-memoize').memoize;

const state1 = {
  booksState: {
    booksObject: {
      'book1-name': {
        shortName: 'book1-name',
      },
    },
  },
  extendedBooksState: {
    extendedBooksObject: {
      'book1-name': {
        staticProp: '5',
        priceString: '10',
        book: {
          shortName: 'book1-name',
        },
      },
    },
  },
};

const state2 = {
  booksState: {
    booksObject: {
      'book1-name': {
        shortName: 'book1-name',
      },
    },
  },
  extendedBooksState: {
    extendedBooksObject: {
      'book1-name': {
        staticProp: '5',
        priceString: '0',
        book: {
          shortName: 'book1-name',
        },
      },
    },
  },
};

const state3 = { ...state1 }; // if use state1 then it will not work

const selectPriceString = memoize((
  state,
) => state.extendedBooksState.extendedBooksObject['book1-name'].priceString);

const selectAdjustedPriceString = memoize((state) => {
  const {
    booksState: {
      booksObject,
    },
    extendedBooksState: {
      extendedBooksObject,
    },
  } = state;

  const book = booksObject['book1-name'];

  book.shortName; // touch the prop

  const priceString = selectPriceString(state);

  extendedBooksObject['book1-name'].staticProp; // touch the prop

  return priceString;
});

const selectAllExtendedBooks = memoize((
  state,
) => Object.values(state.extendedBooksState.extendedBooksObject));

const selectBookThroughExtendedBooks = memoize((state) => selectAllExtendedBooks(state)[0].book);

const selectMemoizedPriceString = memoize((state) => {
  const {
    booksState: {
      booksObject,
    },
  } = state;
  const book = booksObject['book1-name'];

  const priceString = selectPriceString(state);

  book.shortName // touch the prop just in case

  return priceString;

});

const getPriceStringSelector = (book) => (
  state,
) => {
  book.shortName; // touch the prop just in case
  return selectMemoizedPriceString(state);
};

console.log(selectAdjustedPriceString(state1));
getPriceStringSelector(selectBookThroughExtendedBooks(state1))(state1);

getPriceStringSelector(selectBookThroughExtendedBooks(state2))(state2);
console.log(selectAdjustedPriceString(state2));

getPriceStringSelector(selectBookThroughExtendedBooks(state3))(state3);
console.log(selectAdjustedPriceString(state3));

Third expected output is 10

Strange that if use state1 instead of state3 then it has affect

[dai-shi]

Okay, let me merge #101 and tackle the new one separately.

Simplified example

Can you simplify it a bit more?
A test case should be small, and a minimal reproduction helps to investigate the bug.

[Bardiamist]

I simplified as possible

[dai-shi]

Yeah, but my gut feeling is there should be a smaller repro to reveal the bug. Maybe, it has to be created from a different angle, instead of simplifying the current code.

[dai-shi]

I haven't read it very carefully yet, but one possibility is double nested memoize works, but triple nested memoize can fail.

[Bardiamist]

Yeah, but my gut feeling is there should be a smaller repro to reveal the bug.

You can try to remove something then bug will stop to reproduce

Maybe, it has to be created from a different angle, instead of simplifying the current code.

Rare case. Was difficult to find related selectors and call in right order. Simplified as possible and anonymized data.

I haven't read it very carefully

Should be easy to read on books example

one possibility is double nested memoize works, but triple nested memoize can fail.

Better if selectors will be recalled than return wrong data

[dai-shi]

You can try to remove something then bug will stop to reproduce

Done: #102

[dai-shi]

Please open a new issue if you still find a bug.

[Bardiamist]

Cool, I tested, all problems fixed, looks good, thank you! 🎉

[dai-shi]

That's great to hear.

FYI, here's the simplified repro:

proxy-memoize/tests/issue_100.spec.ts

Lines 53 to 87 in e2aa105

	const state1: State = {
	book1: {
	staticProp: '5',
	priceString: '10',
	},
	};
	const state2: State = {
	book1: {
	staticProp: '5',
	priceString: '20',
	},
	};
	const selectBook1 = memoize((state: State) => state.book1);
	const selectPriceString = memoize(
	(state: State) => state.book1.priceString,
	);
	const selectAdjustedPriceString = memoize((state: State) => {
	const priceString = selectPriceString(state);
	state.book1.staticProp; // touch the prop
	return priceString;
	});
	const selectMemoizedPriceString = memoize((state: State) =>
	selectPriceString(state),
	);
	selectBook1(state1);
	selectMemoizedPriceString(state1);
	expect(selectAdjustedPriceString(state1)).toBe('10');
	expect(selectAdjustedPriceString(state2)).toBe('20');