fix: add typings to v1 and v1beta in firestore.d.ts #1433
Conversation
google-cla
bot
added
the
cla: yes
product-auto-label
bot
added
the
api: firestore
Codecov Report
@@ Coverage Diff @@
## master #1433 +/- ##
=======================================
Coverage 98.20% 98.20%
=======================================
Files 32 32
Lines 19502 19502
Branches 1363 1363
=======================================
Hits 19152 19152
Misses 346 346
Partials 4 4 Continue to review full report at Codecov.
|
thebrianchen
force-pushed
the
bc/types-copy
branch
from
28b7026 to
ed94657
Compare
thebrianchen
force-pushed
the
bc/types-copy
branch
from
dab4a57 to
b5dcb0f
Compare
types/firestore.d.ts
Outdated
| @@ -1964,13 +1980,16 @@ declare namespace FirebaseFirestore { | |||
| * Firestore v1beta1 RPCs. | |||
| * @deprecated Use v1 instead. | |||
| */ | |||
| export const v1beta1: any; | |||
| export const v1beta1: typeof import('./v1beta1/firestore_client'); | |||
There was a problem hiding this comment.
You can just do a regular import:
import v1beta1 from '...';
import {FirestoreClient, FirestoreAdminClient} from '...'
There was a problem hiding this comment.
Approach LGTM. Some improvement suggestions.
I also wonder whether this should be a
fixor even afeatsince this is something that we want to create a release for,
'fix' should cause a patch release.
There was a problem hiding this comment.
I don't think top level imports in .d.ts files are allowed (see stackoverflow). I played around with it, but it always ended in sadness.
There was a problem hiding this comment.
Update: Following our conversation, changed to use import directly inline, rather than use typeof.
| * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. | ||
| * See the License for the specific language governing permissions and | ||
| * limitations under the License. | ||
| */ |
There was a problem hiding this comment.
There are now two copyright headers.
thebrianchen
changed the title
…e into bc/types-copy
types/firestore.d.ts
Outdated
| @@ -1964,13 +1962,16 @@ declare namespace FirebaseFirestore { | |||
| * Firestore v1beta1 RPCs. | |||
| * @deprecated Use v1 instead. | |||
| */ | |||
| export const v1beta1: any; | |||
| export const v1beta1: import('./v1beta1/firestore_client').FirestoreClient; | |||
There was a problem hiding this comment.
I am a bit torn about using the default export here, which is really an anti-pattern and not used in the export below. I think no matter how we export this, we have a chance of breaking users, but the clean way would be to export this as { FirestoreClient: import(...).FirestoreClient }. That both works is technical debt that I would like to not carry forward.
Add external typings for
v1andv1beta1infirestore.d.tsfiles by copying over the generated.d.tsfiles intotypes/.Fixes #1422.