Updated version that works with npm/browserify, and fixes unnecessary purging #17
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Using a simple mockup to be able to test local storage in mocha. The test cases are adapted (simplified) from the old test cases that were removed in a previous commit.
Makes the default constructor work with browserify.
Other storage backends need to be required and instantiated:
```
// before
new Cache(maxSize, false, 'local_storage')
// after
CacheStorage = require('cachai/lib/storage/local_storage')
new Cache(maxSize, false, new CacheStorage())
```
Instead of setting auxiliary timers to drive the tests, having to use 'done' callback to avoid silent failures. This is simpler to read, and should give better error messages when tests fail.
If the current size of the cache is larger than the new max size, we call purge. It's not really necessary to check if the new size was larger than the old size before checking the current size. The advantage is that the test is now identical to setItem(), so it can be generalized.
Purging in resize() is now deferred, like in setItem()
When inserting many items in one synchronous "thread", and the cache max size is exceeded, make sure we only set the timeout to purge the cache once!
Updated version that works with browserify, and fixes unnecessary purging
|
Please, by all means, feel free to take this project and run with it. What do you need from me to move forward? Thanks for reaching out! |
|
I'm using this in one of my projects. Will this particular github repository receives updates from now on? And should I change the attribution in my project? |
|
@rstuven wanna ask @monsur to give up the jscache npm repo name so we can get https://github.com/rstuven/node-cachai co-branded as |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
The purpose of this PR is to see if there is anyone paying attention to this project.
We're been using (a copy of) this in our software for several years. Now we need to fix a couple of issues, and would like to use it as a proper npm package, using browserify.
I found rstuven's fork that has already done some of the work of porting it to npm. I've re-purposed this for browserify, by removing the "dynamic" require calls, and added new tests for the localStorage backend.
I've also fixed an issue with the purge function being called many times when adding many items to a full cache.
Now, I would like to start pushing this to npm, and maybe set up a travis-ci job for running the tests. The question is, would it be Ok if I just went ahead and published this as "cachai" on npm, or do you want to hold on to the maintainer status, as the original author of the project?