refactor(storage)!: migrate to TypeScript and match firebase-js-sdk API#8824
refactor(storage)!: migrate to TypeScript and match firebase-js-sdk API#8824russellwheatley merged 120 commits intomainfrom
Conversation
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
|
Hello 👋, this PR has been opened for more than 14 days with no activity on it. If you think this is a mistake please comment and ping a maintainer to get this merged ASAP! Thanks for contributing! You have 7 days until this gets closed automatically |
![cursor[bot]](https://avatars.githubusercontent.com/in/1210556?s=60&v=4){kind=small}
There was a problem hiding this comment.
Cursor Bugbot has reviewed your changes and found 3 potential issues.
❌ Bugbot Autofix is OFF. To automatically fix reported issues with cloud agents, enable autofix in the Cursor dashboard.
Reviewed by Cursor Bugbot for commit 3ee0a27. Configure here.
this was needed for react-native-macos immediately as it runs on 'latest-stable' in CI, while react-native was pinned to a prior version so CI kept passing local builds will fail without this though and do need it
…at still have methods (such as StorageReference) and fire console warning incorrectly
|
You have used all of your free Bugbot PR reviews. To receive reviews on all of your PRs, visit the Cursor dashboard to activate Pro and start your 14-day free trial. |
mikehardy
left a comment
mikehardy
left a comment
There was a problem hiding this comment.
Not as straightforward as app-distribution but still ... reasonably straightforward
some notes as I reviewed, but really only one question on the inclusion of app-check and crashlytics items in an unexpected spot. Resolve that reasonably + merge at will IMHO
| ListResult, | ||
| ListOptions, | ||
| } from './storage'; | ||
| import type EventEmitter from 'react-native/Libraries/vendor/emitter/EventEmitter'; |
There was a problem hiding this comment.
just noting for ambient awareness - react-native is working really hard to define an exported public API and get people to stop calling into the internals (e.g. Libraries/..etc. and we're going to need to clean this up very soon - either current react-native release or in the near future...
There was a problem hiding this comment.
If you know of an issue tracked and have it handy, put in slack channel, please. I'm thinking of having something that tracks important issues (that have an effect on us maintainers) that posts notifications on slack and this seems like a good use case.
|
Approval has been granted pending feedback is resolved 🙏 |
2 participants
Description
types/namespaced.tsand are almost identical to previous index.d.ts. This make it a really easy clean up when we switch off namespaced.Breaking Changes
storage.statics.*types from modular. Need to import individually. See commitRelated issues
Release Summary
Checklist
AndroidiOSOther(macOS, web)e2etests added or updated inpackages/\*\*/e2ejesttests added or updated inpackages/\*\*/__tests__Test Plan
Think
react-native-firebaseis great? Please consider supporting the project with any of the below:React Native FirebaseandInvertaseon TwitterNote
High Risk
High risk because it introduces breaking API renames (
StorageReference→Reference) and reshapes core Storage task/list/ref behaviors and types, which can affect most Storage consumers at compile time and runtime.Overview
Refactors
@react-native-firebase/storageto TypeScript and reorganizes exports around a newtypes/storage.tsmodular surface (with separatetypes/namespaced.tskept for backwards compatibility).Aligns the modular API more closely with the firebase-js-sdk:
ref()now supports fullgs:///https://URLs (replacingrefFromURLusage),child()and modulartoString()helpers are removed in favor ofref(refOrStorage, path)overloads andReference.toString(), and list option typing/handling is updated to allow nullable fields in modular calls.Renames the public reference class from
StorageReferencetoReference, updates task/list internals (StorageTask,StorageListResult) to typed implementations, and adjusts task/error typing to useNativeFirebaseErrorplus asyncpause()/resume()/cancel()semantics. CI type-drift checking is extended to include Storage via new compare-typesstorage-js-sdk.d.tsandpackages/storage/config.ts.Reviewed by Cursor Bugbot for commit 3ee0a27. Bugbot is set up for automated code reviews on this repo. Configure here.