New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
refactor(deprecation)!: remove legacy support for array arguments #625
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think this looks good but maybe we need to update a couple of the jsdoc parameter definitions to match?
const elements: string[] = Array.isArray(segments[0]) | ||
? ((segments[0] as unknown) as string[]) | ||
: segments; | ||
if (Array.isArray(segments[0])) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Remove string[]
from the param type definition above?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Done. Also removed "undocumented" from the PR description...
@@ -433,30 +433,23 @@ export function parseGetAllArguments( | |||
let documents: DocumentReference[]; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I can't comment on unchanged code, but does the getAll() parameter definition need to be updated somehow?
* @param {Array.<DocumentReference|ReadOptions>} documentRefsOrReadOptions
* The `DocumentReferences` to receive, followed by an optional field
* mask.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yes. I fixed the JSDoc as well.
Codecov Report
@@ Coverage Diff @@
## master #625 +/- ##
==========================================
- Coverage 61.6% 61.57% -0.03%
==========================================
Files 21 21
Lines 3422 3420 -2
Branches 459 459
==========================================
- Hits 2108 2106 -2
Misses 1252 1252
Partials 62 62
Continue to review full report at Codecov.
|
Codecov Report
@@ Coverage Diff @@
## master #625 +/- ##
=========================================
Coverage ? 61.32%
=========================================
Files ? 21
Lines ? 3405
Branches ? 460
=========================================
Hits ? 2088
Misses ? 1254
Partials ? 63
Continue to review full report at Codecov.
|
BREAKING CHANGE: This removes the ability to call getAll() and
new FieldPath()
with an array as its first argument instead of calling it with a list of arguments.