Clearer documentation of store contract

[trbrc]

I've been scratching my head trying to understand the ins and outs of the store contract, and how to implement custom stores. This PR is a suggestion for how to improve the documentation.

1. I've expanded the svelte/store intro to be much more explicit about the store contract documentation.
2. I've reorganized the section in 01-component-format.md to describe the syntax first (which is what the headline promises) and the contract second.
3. I put the specifics of what you need to fulfil to implement a store in a numbered list, to make it super clear.
4. I've added a tiny example of how to wrap location.hash and the hashchange event in a custom store. I'm not sure if this is the best example, but it is small. It might be better to include an example of, say, RxJS, but I'm probably not the right person to write it.

[Conduitry]

Made a few few comments, but overall this looks great - thank you!

[Conduitry]

It would be more in the Svelte-y spirit to have this store not actually register the hashchange event handler until the first subscriber is added, and to unregister it when the last subscriber is removed. This can be easily done by checking subscriptions.length before the push and after the splice.

[Conduitry]

What specifically is this trying to address? I think the built-in writable stores will call subscribers again if you .set them to their current value, so it might be a bit confusing if this custom store didn't.

[trbrc]

Setting location.hash triggers the hashchange event asynchronously, so without this check every .set() would call the subscribers twice.

[trbrc]

Let's consider what we want the store contract code example to illustrate.

I wanted to find an example that would not be too complicated, so it could focus on the essentials of implementing the contract rather than the getting bogged down in the specifics of the example.

While it would certainly be more diligent to add and remove the hashchange event handler as needed, I feel like it dilutes the example a bit:

const hashStore = {
	subscribe(subscription) {
		subscription(location.hash); // Call subscription with current value
		if (subscriptions.length === 0) {
			// On the first subscription, set up a `hashchange` event listener,
			// to keep up with browser history or other external hash changes
			window.addEventListener('hashchange', callSubscriptions);
		}
		subscriptions.push(subscription); // Subscribe to future values
		return () => { // Return an "unsubscribe" function
			const index = subscriptions.indexOf(subscription);
			if (index !== -1) {
				subscriptions.splice(index, 1); // Remove the subscription
				if (subscriptions.length === 0) {
					// When no subscriptions are left, remove the `hashchange` listener
					window.removeEventListener('hashchange', callSubscriptions);
				}
			}
		};
	},
	// ...
};

I'm not necessarily against it, but I think it might be more valuable to have a clear and general example than a perfect implementation of a hash store. It works the same either way, if someone copy/pastes the snippet to play around with it.

In the same spirit, I want to follow up on my reply to your comment about the lastHash check. To follow the letter of the store contract the code tries to illustrate, .set needs to call callSubscriptions synchronously. Together with the hashchange event listener, this causes callSubscriptions to be called twice, which makes the store feel broken.

How important is the requirement that subscriptions be called synchronously? The store works without it, and it would allow us to skip the lastHash check and rely on the event listener to do the job. One possibility is to remove this requirement from the contract, but maybe that would cause race conditions when used together with other stores.

How important is it that you can call .set multiple times with the same value and re-trigger subscriptions? Should it be considered part of the contract, or just the way svelte/store happens to work?

I haven't found an elegant and reliable way to only react to events when they're not triggered by the location.hash assignment in .set. Anything I come up with would either be prone to race conditions or make the example too verbose.

[trbrc]

Maybe this is an OK solution:

/* Example: a custom writable store for `location.hash` */

const subscriptions = [];

let storeValue = location.hash; // Set initial value

const hashStore = {
  subscribe(subscription) {
    subscription(storeValue); // Call subscription with current value
    subscriptions.push(subscription); // Subscribe to future values
    return () => { // Return an "unsubscribe" function
      const index = subscriptions.indexOf(subscription);
      if (index !== -1) {
        subscriptions.splice(index, 1); // Remove the subscription
      }
    };
  },
  set(hash) {
    storeValue = hash; // Set the value
    location.hash = hash; // Update location.hash
    for (const subscription of subscriptions) {
      // Call all subscriptions
      subscription(storeValue);
    }
  }
};

// Event listener to stay in sync with external hash changes
window.addEventListener('hashchange', () => {
  if (location.hash !== storeValue) {
    hashStore.set(location.hash);
  }
});

I think that's a very clear example, at the expense of not adding/removing the event listener as needed.

[arlac77]

Why not using a Set instead of Array for subscriptions ?

Adding a subscription
replace

subscriptions.push(subscription)

with

subscriptions.add(subscription)

remove a subscription
replace

const index = subscriptions.indexOf(subscription);
      if (index !== -1) {
        subscriptions.splice(index, 1);
      }

with

subscriptions.delete(subscription)

set value of subscriptions
replace

for (const subscription of subscriptions) {
      subscription(storeValue);
    }

with

subscriptions.forEach(subscription => subscription(storeValue));

[trbrc]

@arlac77 I had actually reached the exact same conclusion 😊 See eaa3416 (it also wraps the function in a unique object, to handle the case where multiple subscriptions is created for the same function).

[Conduitry]

I am kind of thinking now that the API docs are the wrong place to jam in a whole custom store implementation. The rest of the changes look great though, and we could get them in a lot faster if we don't have to worry about figuring out the example.

You had mentioned in chat about maybe making the existing custom store example more helpful and linking to that. I don't think that's a bad idea, but I do think there are two different 'custom store' concepts we ought to be conveying. One is a store based on the exports from svelte/store which creates a store, destructs the methods, and mixes in some new methods to make a store. The other is a completely custom store built from scratch. The second might make people understand the store contract better, but I don't want to lose an example of the first one, because that's probably how most people's custom stores are going to be implemented.

tl;dr - If you don't mind, I'd like to just remove the example for now so we can get this in, and think more about the example or examples later.

[trbrc]

@Conduitry Of course! Let's continue discussing it in an issue.

I don't know if you noticed that I add a little type signature (0193422). It might be enough extra help for others who want to take a stab at making their own stores from scratch.

[thojanssens]

This is gold. And it's hardly available. Would have been nice, to not bloat the docs, is to have a link that will show/hide content. So if you click on "Show example of a store contract implementation", this code expands.