lindelwa122 / dom-wizard

JavaScript library that makes DOM manipulation easy and more managable.
MIT License
7 stars 8 forks source link

createStore Feature #22

Closed sinster2003 closed 9 months ago

sinster2003 commented 9 months ago

Hi @lindelwa122 , so I have worked on this issue, the approach goes as follows:

Go through the documentation once

sinster2003 commented 9 months ago

@lindelwa122 I will start working on getStore function if any improvements needed feel free.

lindelwa122 commented 9 months ago

Hey, @sinster2003, I appreciate your implementation. Please make the following improvements before I can accept your PR:

  1. Ensure that private methods like _store are not exposed. Users should be able to access the store only through public methods like getState().
  2. I realize there might be a flaw in my initial logic while stating the function's requirements. If we remove the store when the page is reloaded, users may lose access to information stored prior to the reload. How about allowing users to invoke createStore as many times as they need? If the store already exists, the function should then add new variables to the store and overwrite existing ones with the same names. What are your thoughts on this change?
sinster2003 commented 9 months ago

Hey @lindelwa122 I have figured out the changes that can be made ...

  1. I have handled the refresh or reload exception with the flag ensuring no data loss at all.
  2. createStore is used to just create store, the store won't be exposed the user can only retrieve the store for update and getState only using getStore()
  3. I had to implement getStore because of the this pointer in both getState and updateState we need an object reference of store.

Something like this:

createStore(store);

const data = getStore(); // gets the store from anywhere

console.log(data); // retrives store

console.log(data.getState("name")); // gets the property

const newdata = data.updateState("name", "akash"); // updates the state of store

console.log("New data", newdata); // new updated state
  1. I will be making a PR request commit soon

I think your previous thought process of createStore was absolutely right making it invokable only once and i feel that implementation is better than overwriting the store again and again, conceptually it doesn't sound good. If i give overwriting as the primary feature I just feel the importance of updateState will decrease multifolds. So we will proceed with the previous approach. I tested the changed code and it did work.
If changes needed do feel free.

sinster2003 commented 9 months ago

I will resolve the branch conflicts and make a PR