basil.js icon indicating copy to clipboard operation
basil.js copied to clipboard

Issue with namespace "reset" logic

Open Pranjali14 opened this issue 4 years ago • 3 comments

Steps to reproduce -

  • Create two namespaces for the same application namely - "namespace" and "namespacePersistent".
  • Try to reset "namespace".
  • Observe that even "namespacePersistent" gets reset.

I went through the code and observed that the way the "reset" function is implemented

reset: function (namespace) {
				for (var i = 0, key; i < window[this.engine].length; i++) {
					key = window[this.engine].key(i);
					if (!namespace || key.indexOf(namespace) === 0) {
						this.remove(key);
						i--;
					}
				}
	}

In this case key.indexOf(namespace) === 0 for namespace and namespacePersistent will be true and hence will reset both the namespaces.

Solution : We can use a combination of namespace and delimiter for finding the index. Considering the default delimiter as ' . ' a namespace delimiter combination will not return 0

let key = 'namespacePersistent.key1' ;
key.indexOf("namespace.") === 0 // false since index returned will be -1.

We can pass the namespace and delimiter combination from the callee.

reset: function (options) {
				options = Basil.utils.extend({}, this.options, options);
				Basil.utils.tryEach(_toStoragesArray(options.storages), function (storage) {
                                           let namespaceStr = options.namespace+(options.keyDelimiter || '.'); 
					_storages[storage].reset(namespaceStr);
				}, null, this);
			},

If this solution is looks fine I can raise a PR for the same.

Pranjali14 avatar May 19 '20 14:05 Pranjali14

Hi @Pranjali14

Thanks for this issue, very clear and even with envisaged solution :)

Indeed, we could easily replace key.indexOf(namespace) === 0 by key.indexOf(namespace + (options.keyDelimiter || '.' )) === 0 to ensure being more strict and make your case working.

Would you made making a PR with that change, and if possible, even with a test in the test suite (with your above example?)

Thanks

guillaumepotier avatar May 19 '20 15:05 guillaumepotier

Sure @guillaumepotier will make the changes and update the test suite as well.

Thanks

Pranjali14 avatar May 19 '20 16:05 Pranjali14

Hi @guillaumepotier,

I have raised a PR for this issue.

Thanks, Pranjali

Pranjali14 avatar Jun 15 '20 04:06 Pranjali14