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

ADD: cache parseUserAgent result to increase performance #139

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

Gsiete
Copy link

@Gsiete Gsiete commented May 24, 2020

Many users of the library might be calling the detect function directly in React render functions or other pieces of code that gets rerun many times. Since usually the browser running the detect function will not change it's user agent, it's worth it to save the already parsed userAgents to avoid parsing the same one multiple times.

Many users of the library might be calling the detect function directly in React render functions or other pieces of code that gets rerun many times. Since usually the browser running the detect function will not change it's user agent, it's worth it to save the already parsed userAgents to avoid parsing the same one multiple times.
@DamonOehlman
Copy link
Owner

DamonOehlman commented May 24, 2020

@Gsiete I'm not sure about adding this functionality into the library. I can definitely see why you have raised a PR for it, but in most cases I'd consider caching to be the responsibility of another library.

In the cases of React I guess I'd probably be thinking I'd be reaching for the useMemo hook myself, and in a node environment probably something like lru-cache.

Basically, I'd like for people to be able to choose their own caching strategy rather than detect-browser implement something. Happy to have more discussion around this and leave the PR open for others to share their thoughts also.

@Gsiete
Copy link
Author

Gsiete commented May 25, 2020

@DamonOehlman I see.
My view was that the library is in grade of making an absolute decision on where the caching can go based on the which function is a pure function (parseUserAgent will never give a different result with the same input) and which isn't (detect isn't it, since when called without arguments it's result depends on navigator.userAgent).
In other words, a developer that wants to use detect() without being aware of it's internal workings wouldn't really know what to depend useMemo on.
Of course, he could calculate it once, but when emulating other devices in Chrome it wouldn't work properly.
I think that what I'm trying to say is that I think that probably many people using the library wouldn't really know which caching strategy to use (or think about it at all), whereas the library can more easily take a decision.

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

Successfully merging this pull request may close these issues.

None yet

2 participants