fix: Change default storage type, migrate existing users #875
Conversation
| return { | ||
| initialPathName: window.location.pathname, | ||
| referringDomain: _info.referringDomain(), | ||
| ..._info.campaignParams(), |
There was a problem hiding this comment.
this also means we're not storing extra stuff beyond the keys I actually use - actually the biggest space saving comapred to reducing the key lengths
|
Size Change: +3.59 kB (0%) Total Size: 749 kB
ℹ️ View Unchanged
|
src/session-props.ts
Outdated
| const campaignParams = _info.campaignParams() | ||
| return _strip_empty_properties({ | ||
| p: window.location.pathname, | ||
| r: _info.referringDomain(), | ||
| m: campaignParams.utm_medium, | ||
| s: campaignParams.utm_source, | ||
| c: campaignParams.utm_campaign, | ||
| n: campaignParams.utm_content, | ||
| t: campaignParams.utm_term, |
There was a problem hiding this comment.
This won't actually help that much... 4Kb is not a lot especially as urls and things will be pretty long.
The main reason we see this issue on posthog.com is that we are not using localstorage+cookie persistence so I think this error can happen a lot anyways.
There was a problem hiding this comment.
Every little helps 🤷 - but yeah the main thing is using the right storage engine, and the other main thing is not storing values I don't need
There was a problem hiding this comment.
fly-by without having read or thought about the change much...
It would be nice to have the saving in the cookie without needing to read the tiny identifiers when being a human looking at the code. Feels easy to break something because there's not much for your brain to hold on to when reading it.
|
Converted to draft while I add the store migration |
robbie-c
force-pushed
the
reduce-storage-size
branch
from
29257fb
to
a125c33
Compare
robbie-c
changed the title
|
This PR hasn't seen activity in a week! Should it be merged, closed, or further worked on? If you want to keep it open, post a comment or remove the |
robbie-c
force-pushed
the
reduce-storage-size
branch
from
a125c33
to
bc62de0
Compare
See #876
Follow up to #869
Changes
Move the session-scoped props to smaller keys in storage. Not really the right fix (which would be moving them to a separate cookie) but at least a quick fix.
Also limit the keys stored to only the ones actually used. This is quite a dramatic change, so might be enough on its own to avoid reworking the storage for now
Checklist