refactor: allow @netlify/blobs v9 and v10 as peer dependency#640
refactor: allow @netlify/blobs v9 and v10 as peer dependency#640
@netlify/blobs v9 and v10 as peer dependency#640Conversation
Breaking changes are compatible with unstorage: - v9: https://github.com/netlify/primitives/releases/tag/blobs-v9.0.0 - v10: https://github.com/netlify/primitives/releases/tag/blobs-v10.0.0
|
Nice! Anything left for draft PR? (we should check failing ci) |
@netlify/blobs v9 and v10@netlify/blobs v9 and v10 as peer dependency
|
Pushed commit to fix types |
|
Yeah, I guess I need to adjust the netlify blobs driver options type to support some new options added in v10. I'll get to that soon. |
|
oh heh I see you just pushed! I think we still need the driver options type to be compatible with all supported @netlify/blobs versions. We already have a pattern there with a union type. We just need to add to it. This is the weird thing with peer dependencies... and CI won't capture this because we only (implicitly) test type compat via the calls in the test suite, which use only the dev dep version. 🤷🏼 |
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## main #640 +/- ##
==========================================
+ Coverage 59.99% 61.34% +1.34%
==========================================
Files 42 42
Lines 3657 3795 +138
Branches 590 632 +42
==========================================
+ Hits 2194 2328 +134
- Misses 1460 1464 +4
Partials 3 3 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
| // TODO(serhalp): Allow drivers to return a value from `setItem`. The @netlify/blobs v10 | ||
| // functionality isn't usable without this. |
There was a problem hiding this comment.
Let's get this merged and released as is for now, but it would be great to discuss this afterwards!
There was a problem hiding this comment.
Can you please explain why the functionality isn't usable? Aren't keys in netlify blobs predictable?
There was a problem hiding this comment.
Everything will "work" fine—it just won't be possible to leverage the new functionality in v10 because the user won't be able to access the return value {modified, etag} from set() and setJSON().
I think this is just a driver interface typing issue. It's marked as returning Promise<void> but we would need it to be something like <T = void>...Promise<T>.
There was a problem hiding this comment.
I see. Currently the interface is intentionally making sure bo return value so that any usage of drivers wont be vendor/driver specific and switching seamless.
We might later add vendor prefixed namespaces for users that really know what they do or a new API set for blobs (some providers don’t even have predictable key after upload it makes sense for them to return meta on set)
|
@pi0 I saw you changed this to |
@netlify/blobs v9 and v10 as peer dependency@netlify/blobs v9 and v10 as peer dependency
|
Maybe longer term, we can have a scheduled monthly workflow or something which force installs the various versions of peers we have, including latest To then run regular tests We don't need it on every ci build but it could help us catch upcoming compat problems etc too |
Breaking changes are compatible with unstorage:
This also bumps the dev dep to latest while I'm at it.