Skip to content

Review from Tom #8983

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.

Sign up for GitHub

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

Open
wants to merge 6 commits into
base: feat/pipelines
Choose a base branch
from
+33 −28
Open

Review from Tom #8983

Changes from all commits
Commits
Show all changes
6 commits
Select commit Hold shift + click to select a range
508d79d
Fixes
tom-andersen Apr 29, 2025
cf1c4b0
Fixes
tom-andersen Apr 29, 2025
fe811df
Fixes
tom-andersen Apr 30, 2025
ededa71
Fixes
tom-andersen May 1, 2025
a5ec03b
Fixes
tom-andersen May 1, 2025
dfe38a4
Fixes
tom-andersen May 2, 2025
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
61 changes: 33 additions & 28 deletions packages/firestore/src/lite-api/expressions.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -698,7 +698,8 @@ export abstract class Expr implements ProtoValueSerializable, UserData {
}

/**
* Creates an expression that checks if a field exists in the document.
* Creates an expression that checks if this expression evaluates to a name of
* the field that exists.
*
* ```typescript
* // Check if the document has a field named "phoneNumber"
Expand Down Expand Up @@ -1889,7 +1890,7 @@ export abstract class Expr implements ProtoValueSerializable, UserData {
* ```
*
* @param key The name of the key to remove from the input map.
* @returns A new {@code FirestoreFunction} representing the 'mapRemove' operation.
* @return A new {@code Expr} that evaluates to a modified map.
*/
mapRemove(key: string): FunctionExpr;
/**
Expand All @@ -1903,7 +1904,7 @@ export abstract class Expr implements ProtoValueSerializable, UserData {
* ```
*
* @param keyExpr An expression that produces the name of the key to remove from the input map.
* @returns A new {@code FirestoreFunction} representing the 'mapRemove' operation.
* @return A new {@code Expr} that evaluates to a modified map.
*/
mapRemove(keyExpr: Expr): FunctionExpr;
mapRemove(stringExpr: Expr | string): FunctionExpr {
Expand Down Expand Up @@ -3139,30 +3140,30 @@ export function isAbsent(value: Expr | string): BooleanExpr {
/**
* @beta
*
* Creates an expression that checks if an expression evaluates to 'NaN' (Not a Number).
* Creates an expression that checks if tbe result of an expression is null.
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The example needs to be fixed

*
* ```typescript
* // Check if the result of a calculation is NaN
* isNaN(field("value").divide(0));
* ```
*
* @param value The expression to check.
* @return A new {@code Expr} representing the 'isNaN' check.
* @return A new {@code BooleanExpr} representing the 'isNull' check.
*/
export function isNull(value: Expr): BooleanExpr;

/**
* @beta
*
* Creates an expression that checks if a field's value evaluates to 'NaN' (Not a Number).
* Creates an expression that checks if tbe value of a field is null.
*
* ```typescript
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The example needs to be fixed

* // Check if the result of a calculation is NaN
* isNaN("value");
* ```
*
* @param value The name of the field to check.
* @return A new {@code Expr} representing the 'isNaN' check.
* @return A new {@code BooleanExpr} representing the 'isNull' check.
*/
export function isNull(value: string): BooleanExpr;
export function isNull(value: Expr | string): BooleanExpr {
Expand All @@ -3180,7 +3181,7 @@ export function isNull(value: Expr | string): BooleanExpr {
* ```
*
* @param value The expression to check.
* @return A new {@code Expr} representing the 'isNaN' check.
* @return A new {@code BooleanExpr} representing the 'isNotNull' check.
*/
export function isNotNull(value: Expr): BooleanExpr;

Expand All @@ -3195,7 +3196,7 @@ export function isNotNull(value: Expr): BooleanExpr;
* ```
*
* @param value The name of the field to check.
* @return A new {@code Expr} representing the 'isNaN' check.
* @return A new {@code BooleanExpr} representing the 'isNotNull' check.
*/
export function isNotNull(value: string): BooleanExpr;
export function isNotNull(value: Expr | string): BooleanExpr {
Expand All @@ -3213,7 +3214,7 @@ export function isNotNull(value: Expr | string): BooleanExpr {
* ```
*
* @param value The expression to check.
* @return A new {@code Expr} representing the 'isNotNaN' check.
* @return A new {@code BooleanExpr} representing the 'isNotNaN' check.
*/
export function isNotNan(value: Expr): BooleanExpr;

Expand All @@ -3228,7 +3229,7 @@ export function isNotNan(value: Expr): BooleanExpr;
* ```
*
* @param value The name of the field to check.
* @return A new {@code Expr} representing the 'isNotNaN' check.
* @return A new {@code BooleanExpr} representing the 'isNotNaN' check.
*/
export function isNotNan(value: string): BooleanExpr;
export function isNotNan(value: Expr | string): BooleanExpr {
Expand All @@ -3247,6 +3248,7 @@ export function isNotNan(value: Expr | string): BooleanExpr {
*
* @param mapField The name of a field containing a map value.
* @param key The name of the key to remove from the input map.
* @return A new {@code Expr} that evaluates to a modified map.
*/
export function mapRemove(mapField: string, key: string): FunctionExpr;
/**
Expand All @@ -3261,6 +3263,7 @@ export function mapRemove(mapField: string, key: string): FunctionExpr;
*
* @param mapExpr An expression return a map value.
* @param key The name of the key to remove from the input map.
* @return A new {@code Expr} that evaluates to a modified map.
*/
export function mapRemove(mapExpr: Expr, key: string): FunctionExpr;
/**
Expand All @@ -3275,6 +3278,7 @@ export function mapRemove(mapExpr: Expr, key: string): FunctionExpr;
*
* @param mapField The name of a field containing a map value.
* @param keyExpr An expression that produces the name of the key to remove from the input map.
* @return A new {@code Expr} that evaluates to a modified map.
*/
export function mapRemove(mapField: string, keyExpr: Expr): FunctionExpr;
/**
Expand All @@ -3289,6 +3293,7 @@ export function mapRemove(mapField: string, keyExpr: Expr): FunctionExpr;
*
* @param mapExpr An expression return a map value.
* @param keyExpr An expression that produces the name of the key to remove from the input map.
* @return A new {@code Expr} that evaluates to a modified map.
*/
export function mapRemove(mapExpr: Expr, keyExpr: Expr): FunctionExpr;

Expand Down Expand Up @@ -4664,7 +4669,7 @@ export function eqAny(
*
* @param fieldName The field to compare.
* @param arrayExpression An expression that evaluates to an array, whose elements to check for equality to the input field.
* @return A new {@code Expr} representing the 'IN' comparison.
* @return A new {@code BooleanExpr} representing the 'IN' comparison.
*/
export function eqAny(fieldName: string, arrayExpression: Expr): BooleanExpr;
export function eqAny(
Expand All @@ -4678,7 +4683,7 @@ export function eqAny(
/**
* @beta
*
* Creates an expression that checks if an expression is not equal to any of the provided values
* Creates an expression that checks if an expression is not equal to all the provided values
* or expressions.
*
* ```typescript
Expand All @@ -4688,7 +4693,7 @@ export function eqAny(
*
* @param element The expression to compare.
* @param values The values to check against.
* @return A new {@code Expr} representing the 'NOT IN' comparison.
* @return A new {@code BooleanExpr} representing the 'NOT IN' comparison.
*/
export function notEqAny(
element: Expr,
Expand All @@ -4698,7 +4703,7 @@ export function notEqAny(
/**
* @beta
*
* Creates an expression that checks if a field's value is not equal to any of the provided values
* Creates an expression that checks if a field's value is not equal to all the provided values
* or expressions.
*
* ```typescript
Expand All @@ -4708,7 +4713,7 @@ export function notEqAny(
*
* @param fieldName The field name to compare.
* @param values The values to check against.
* @return A new {@code Expr} representing the 'NOT IN' comparison.
* @return A new {@code BooleanExpr} representing the 'NOT IN' comparison.
*/
export function notEqAny(
fieldName: string,
Expand All @@ -4718,7 +4723,7 @@ export function notEqAny(
/**
* @beta
*
* Creates an expression that checks if an expression is not equal to any of the provided values
* Creates an expression that checks if an expression is not equal to all the provided values
* or expressions.
*
* ```typescript
Expand All @@ -4728,14 +4733,14 @@ export function notEqAny(
*
* @param element The expression to compare.
* @param arrayExpression The values to check against.
* @return A new {@code Expr} representing the 'NOT IN' comparison.
* @return A new {@code BooleanExpr} representing the 'NOT IN' comparison.
*/
export function notEqAny(element: Expr, arrayExpression: Expr): BooleanExpr;

/**
* @beta
*
* Creates an expression that checks if a field's value is not equal to any of the values in the evaluated expression.
* Creates an expression that checks if a field's value is not equal to all the values in the evaluated expression.
*
* ```typescript
* // Check if the 'status' field is not equal to any value in the field 'rejectedStatuses'
Expand All @@ -4744,7 +4749,7 @@ export function notEqAny(element: Expr, arrayExpression: Expr): BooleanExpr;
*
* @param fieldName The field name to compare.
* @param arrayExpression The values to check against.
* @return A new {@code Expr} representing the 'NOT IN' comparison.
* @return A new {@code BooleanExpr} representing the 'NOT IN' comparison.
*/
export function notEqAny(fieldName: string, arrayExpression: Expr): BooleanExpr;

Expand Down Expand Up @@ -4773,7 +4778,7 @@ export function notEqAny(
* @param first The first condition.
* @param second The second condition.
* @param additionalConditions Additional conditions to 'XOR' together.
* @return A new {@code Expr} representing the logical 'XOR' operation.
* @return A new {@code BooleanExpr} representing the logical 'XOR' operation.
*/
export function xor(
first: BooleanExpr,
Expand Down Expand Up @@ -4951,7 +4956,7 @@ export function logicalMinimum(
* ```
*
* @param value An expression evaluates to the name of the field to check.
* @return A new {@code Expr} representing the 'exists' check.
* @return A new {@code BooleanExpr} representing the 'exists' check.
*/
export function exists(value: Expr): BooleanExpr;

Expand All @@ -4966,7 +4971,7 @@ export function exists(value: Expr): BooleanExpr;
* ```
*
* @param fieldName The field name to check.
* @return A new {@code Expr} representing the 'exists' check.
* @return A new {@code BooleanExpr} representing the 'exists' check.
*/
export function exists(fieldName: string): BooleanExpr;
export function exists(valueOrField: Expr | string): BooleanExpr {
Expand All @@ -4984,7 +4989,7 @@ export function exists(valueOrField: Expr | string): BooleanExpr {
* ```
*
* @param value The expression to check.
* @return A new {@code Expr} representing the 'isNaN' check.
* @return A new {@code BooleanExpr} representing the 'isNaN' check.
*/
export function isNan(value: Expr): BooleanExpr;

Expand All @@ -4999,7 +5004,7 @@ export function isNan(value: Expr): BooleanExpr;
* ```
*
* @param fieldName The name of the field to check.
* @return A new {@code Expr} representing the 'isNaN' check.
* @return A new {@code BooleanExpr} representing the 'isNaN' check.
*/
export function isNan(fieldName: string): BooleanExpr;
export function isNan(value: Expr | string): BooleanExpr {
Expand All @@ -5017,7 +5022,7 @@ export function isNan(value: Expr | string): BooleanExpr {
* ```
*
* @param stringExpression An expression evaluating to a string value, which will be reversed.
* @return A new {@code Expr} representing the reversed string.
* @return A new {@code BooleanExpr} representing the reversed string.
*/
export function reverse(stringExpression: Expr): FunctionExpr;

Expand Down Expand Up @@ -5867,10 +5872,10 @@ export function strConcat(
* ```
*
* @param fieldName The field name of the map field.
* @param subField The key to access in the map.
* @param key The key to access in the map.
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

subField doesn't seem quite right within the context of maps. I kinda see where you are going, in which case maybe subFieldName might be more accurate within the context of documents. I personally prefer the word key since it has good programatic meaning.

* @return A new {@code Expr} representing the value associated with the given key in the map.
*/
export function mapGet(fieldName: string, subField: string): FunctionExpr;
export function mapGet(fieldName: string, key: string): FunctionExpr;

/**
* @beta
Expand Down
Loading