feat(firestore, web): Web implementation for Pipeline APIs#18100
feat(firestore, web): Web implementation for Pipeline APIs#18100SelaseKay wants to merge 12 commits intofirestore-pipelines-dart-api-v2from
Conversation
Using Gemini Code AssistThe full guide for Gemini Code Assist can be found on our documentation page, here are some quick tips. Invoking Gemini You can request assistance from Gemini at any point by creating a comment using either
Customization To customize Gemini Code Assist for GitHub experience, repository maintainers can create a configuration file and/or provide a custom code review style guide (such as PEP-8 for Python) by creating and adding files to a Limitations & Feedback Gemini Code Assist may make mistakes. Please leave feedback on any instances where its feedback is incorrect or counter productive. You can react with 👍 and 👎 on @gemini-code-assist comments. If you're interested in giving your feedback about your experience with Gemini Code Assist for Github and other Google products, sign up here. |
| .toUpper(); | ||
| case 'trim': | ||
| return (_expr(argsMap, _kExpression) as interop.ExpressionJsImpl) | ||
| .toLower(); |
There was a problem hiding this comment.
case 'trim':
return (_expr(argsMap, _kExpression) as interop.ExpressionJsImpl)
.toLower(); // BUG: should be .trim()Copy-paste bug. This will silently produce wrong results — a trim() call will lowercase the string instead of trimming whitespace.
| const firestoreServiceName = 'firestore'; | ||
|
|
||
| if (service.name == firestoreServiceName) { | ||
| // Inject the Firestore Pipelines script. This bundle supports both | ||
| // Pipeline operations (Enterprise edition) and standard Firestore queries. | ||
| return injectSrcScript( | ||
| 'https://www.gstatic.com/firebasejs/$version/firebase-firestore-pipelines.js', | ||
| 'firebase_$firestoreServiceName', | ||
| ); | ||
| } | ||
|
|
There was a problem hiding this comment.
We need to add a TODO or something? is it normal to inject it this way? Will it be supported in the normal bundle?
There was a problem hiding this comment.
https://www.gstatic.com/firebasejs/$version/firebase-firestore-pipelines.js bundle has support for both standard queries and Pipeline Operations. The previous bundle had support for only standard queries.
| /// Converts Dart serialized pipeline expressions/stage args into JS pipeline | ||
| /// types by calling the pipelines interop API (field, constant, equal, and, | ||
| /// ascending, etc.) that mirrors the Firebase JS SDK. | ||
| class PipelineExpressionParserWeb { |
There was a problem hiding this comment.
Are we missing those?
negateas_booleanto_string_with_formatbit_and,bit_or,bit_xor,bit_not,bit_left_shift,bit_right_shiftdocument_id_from_refarray_reversearray_slicedistancetimestamp_diffmap_from_pairs
There was a problem hiding this comment.
Besides array_reverse, document_id_from_ref, and as_boolean the others are not supported on Web
| return pipeline.union(otherPipeline); | ||
| default: | ||
| // Ignore unknown stages | ||
| return pipeline; |
There was a problem hiding this comment.
default:
// Ignore unknown stages
return pipeline;If a stage type is unrecognized, it's silently skipped. This could produce subtly wrong query results that are extremely hard to debug. This should throw an error instead, like:
default:
throw FirebaseException(
plugin: 'cloud_firestore',
message: 'Unknown pipeline stage: ${stage['stage']}',
);| interop.PipelineResultJsImpl jsResult) { | ||
| final d = jsResult.data(); | ||
| return d != null | ||
| ? Map<String, dynamic>.from(dartify(d) as Map<Object?, Object?>) | ||
| : null; |
There was a problem hiding this comment.
If dartify(d) returns null even when d is non-null (e.g. a JS object that can't be converted), the as Map<Object?, Object?> cast will throw. The version in firestore.dart handles this more safely:
final parsed = dartify(d);
return parsed != null ? Map<String, dynamic>.from(parsed as Map<Object?, Object?>) : null;2 participants
Description
Replace this paragraph with a description of what this PR is doing. If you're modifying existing behavior, describe the existing behavior, how this PR is changing it, and what motivated the change.
Related Issues
Replace this paragraph with a list of issues related to this PR from the issue database. Indicate, which of these issues are resolved or fixed by this PR. Note that you'll have to prefix the issue numbers with flutter/flutter#.
Checklist
Before you create this PR confirm that it meets all requirements listed below by checking the relevant checkboxes (
[x]).This will ensure a smooth and quick review process. Updating the
pubspec.yamland changelogs is not required.///).melos run analyze) does not report any problems on my PR.Breaking Change
Does your PR require plugin users to manually update their apps to accommodate your change?