continentaldivide / destiny-status

Player lookup tool for Destiny 2
https://destinystatus.net
0 stars 1 forks source link

Add log of stored manifest tables to keyval #27

Closed continentaldivide closed 1 year ago

continentaldivide commented 1 year ago

The objective of this PR is to solve an oversight with the existing implementation of the manifest:

The browser-stored version of the manifest is only updated when, on page load, the client discovers that Bungie has released a new version of the manifest on their end. This is great, unless the app is updated with a feature that involves storing an additional table from the manifest, such as the change a couple weeks ago that added the DamageTypeDefinition table. In such a case, the client will see that Bungie has not updated the manifest, thus there's no need to re-fetch it, and the page will break as soon as the client encounters code reliant on the missing table.

While this PR solves the problem, I'm not happy with where I'm leaving useManifestStatus -- that code does not spark joy and I think it could really use a refactor. However, as it stands, it seems like that's mostly an aesthetic issue; it's hard to imagine how much more that hook is going to change other than future table additions. So I'm reluctantly calling that YAGNI territory, and if it does turn out that the hook needs to evolve later, the yet-to-be-discovered needs will make it clearer how the code would best be organized.

vercel[bot] commented 1 year ago

The latest updates on your projects. Learn more about Vercel for Git ↗︎

Name Status Preview Updated (UTC)
destiny-status ✅ Ready (Inspect) Visit Preview Sep 14, 2023 4:38am