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

feat(cache): implement caching for intl data #235

Open
wants to merge 2 commits into
base: master
Choose a base branch
from

Conversation

dvlin-dev
Copy link
Contributor

Description

add cache

Fixes #232

Type of change

New feature (non-breaking change which adds functionality)

Test

First call intl.get once, and then check if there is a corresponding value in the cache.

  • Add Test Case: cache

@cwtuan
Copy link
Collaborator

cwtuan commented Oct 7, 2023

First of all, thanks for spending time on this project.

Is it possible to make the cache feature optional by parameter? I would like it to be disabled by default for the following reasons:

  1. The cached messages may consume more memory.
  2. I would like it maintain the same behavior of the previous version.

@cwtuan
Copy link
Collaborator

cwtuan commented Oct 7, 2023

Additionally, is it possible to use the cache mechanism offered by https://formatjs.io/docs/intl/#createintlcache?

@dvlin-dev
Copy link
Contributor Author

I submitted the code again, according to your request, make cache optional, refer to formatjs

@dvlin-dev
Copy link
Contributor Author

The changes to the init method parameters need to be synchronized in the README.md file, just like defined in index.d.ts. However, the README.md file is ignored in the .gitignore file.

Should I delete the README.md option in the .gitignore file, or should you synchronize the changes of the init parameter?

@dvlin-dev
Copy link
Contributor Author

I am very happy to be able to contribute (:

@dvlin-dev
Copy link
Contributor Author

hi bro, maybe you can have a look at of this,I will use the feature plz.

@luxsush
Copy link

luxsush commented Mar 11, 2024

Bump

* @returns {Promise}
*/
init(options = {}) {
init(options = {}, cache) {
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

could you create cache object internally by adding a enableCache as one of options?

init(options) {
  if(options.enableCache) {
    // create cache object internally
  }
}

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.

cache
3 participants