Cache shapes based only on parameters of the shape definition

[balegas]

sortedOptionsHash() uses all ShapeStreamOptions option keys, which prevents deduplication of shapes in shape cache if they have different parameters that don't belong to the shape definition.

My case is that I create a top-level shape with offset and shapeId, but in other places I don't have those parameters and I don't want to thread them through code.

What do you think?

[netlify]

✅ Deploy Preview for electric-next ready!

Name	Link
🔨 Latest commit	9149053
🔍 Latest deploy log	https://app.netlify.com/sites/electric-next/deploys/6721171547550c000870d934
😎 Deploy Preview	https://deploy-preview-1904--electric-next.netlify.app
📱 Preview on mobile	Toggle QR Code... Use your smartphone camera to open QR code link.

---

To edit notification comments on pull requests, go to your Netlify site configuration.

[KyleAMathews]

Ah interesting use case — yeah makes sense — within the context of an app, the shape def is what's a stable pointer to the instantiation of a ShapeStream/Shape.

This would prevent someone from creating a Shape that starts at different offsets but that's such an odd edge case that someone could just skip out of useShape at that point.

[balegas]

Ah interesting use case — yeah makes sense — within the context of an app, the shape def is what's a stable pointer to the instantiation of a ShapeStream/Shape.

Exactly, there are things that are specific of shape instance and not the shape definition. e.g. if you use different abort signals (I don't even know what the code will do in the case of passing an object to the stringify).

I just realised this is buggy already. If we don't want to subscribe to a shape we should never cache it.

[balegas]

• need to handle deletes
• don't cache shape if subsbribe: false

I will need this for the SSR example and I think currently it is sub-optimal. The only question is if we should include headers or not. I think yes, because of auth.

[icehaunter]

Converted to draft until @balegas says it's ready

[balegas]

I will work on this on the related PR. Just wanted to get feedback about the idea