Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

useMemo does not work as described on get properties #246

Open
dark-panda opened this issue Dec 2, 2022 · 4 comments
Open

useMemo does not work as described on get properties #246

dark-panda opened this issue Dec 2, 2022 · 4 comments

Comments

@dark-panda
Copy link

The documentation at https://github.com/stimulus-use/stimulus-use/blob/main/docs/use-memo.md says that useMemo values are memoized on first use, but this is not the case for get properties, which is what the documentation describes. In the case of get properties, the properties are memoized immediately when useMemo is called, which can lead to unexpected results as you may not be expecting that initial before actual usage. The docs would indicate that you need to actually access the property at least once to memoize it, but that's not that the code says: https://github.com/stimulus-use/stimulus-use/blob/main/src/use-memo/use-memo.ts#L10

memoize(controller, getter, (controller as any)[getter])

This is fetching the value immediately, rather than waiting for first use. This can lead to some unexpected results when you have code like this:

static memos = ['memoizedProperty', 'memoizedMethod']

connect() {
  useMemo(this) // this calls memoizedProperty(), but not memoizedMethod()

  this.memoizedPropety // second call, already called via useMemo
  this.memoizedMethod() // first call, is not memoized via useMemo
}

get memoizedProperty() {
  return 'property'
}

memoizedMethod() {
  return 'method'
}
@marcoroth
Copy link
Member

marcoroth commented Jan 12, 2023

Hey @dark-panda, thanks for opening this issue.

I'm not sure if we can also make useMemo work for getters, due to the nature how they work.

We might be able to print out a warning if we detect that you provided a getter in the static memos array and note the (unexpected) behavior in the docs.

Or do you have another idea in mind?

@dark-panda
Copy link
Author

@marcoroth if we can detect that we're using a getter, perhaps we can code around it? Like we could perhaps ignore the first call made to the getter and then start memoizing on any subsequent calls?

@marcoroth
Copy link
Member

I'll look into this to see what we can do 🤞🏼

@marcoroth
Copy link
Member

I think the current implementation doesn't even work for regular methods, just getters 🤔

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants