Release Bundles for Next SDK #4352
Conversation
|
wu-hui
requested review from
Feiyang1,
hiranya911 and
hsubox76
as code owners
Size Analysis Report |
| * @param bundleData | ||
| * An object representing the bundle to be loaded. Valid objects are `ArrayBuffer`, | ||
| * `ReadableStream<Uint8Array>` or `string`. | ||
| * |
There was a problem hiding this comment.
This documentation style (newline versus dash) seems a bit inconsistent.
packages/firestore/export.ts
Outdated
| import { | ||
| loadBundle as expLoadBundle, | ||
| namedQuery as expNamedQuery | ||
| } from './src/exp/database'; |
There was a problem hiding this comment.
Can you move the imports to the top of the file?
Co-authored-by: Sebastian Schmidt <[email protected]>
.changeset/giant-months-guess.md
Outdated
| @@ -0,0 +1,5 @@ | |||
| --- | |||
There was a problem hiding this comment.
You probably don't want Changesets for exp-only changes as they show up in the wrong release notes.
There was a problem hiding this comment.
Right. Let me Remove.
packages/firestore/src/exp/bundle.ts
Outdated
| @@ -46,6 +62,17 @@ export class LoadBundleTask | |||
| documentsLoaded: 0 | |||
| }; | |||
|
|
|||
| /** | |||
| * Registers functions to listen to bundle loading progress events. | |||
| * @param next | |||
There was a problem hiding this comment.
There should be a - after @param next. It is the format required by tsdoc.
| // @public | ||
| export class LoadBundleTask implements PromiseLike<LoadBundleTaskProgress> { | ||
| catch<R>(onRejected: (a: Error) => R | PromiseLike<R>): Promise<R | LoadBundleTaskProgress>; | ||
| _completeWith(progress: LoadBundleTaskProgress): void; |
There was a problem hiding this comment.
I think these _ methods should be tagged @internal in packages/firestore/src/exp/bundle.ts if we don't want to expose them publicly?
There was a problem hiding this comment.
I think @schmidt-sebastian's script strips things with a leading underscore automatically in the d.ts file, but not in the api report.
Ideally this file should be consistent with d.ts. Maybe we can extend the script to update the api report too?
There was a problem hiding this comment.
We don't have to do it in this PR of course.
There was a problem hiding this comment.
Waiting for review :) #4207
No description provided.