Skip to content
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

feat(cloud_firestore): Create a cloud_firestore_types package that can be used in dart-only contexts #13215

Open
wants to merge 3 commits into
base: main
Choose a base branch
from

Conversation

Rexios80
Copy link
Contributor

@Rexios80 Rexios80 commented Aug 25, 2024

Description

I am working on Dart code that gets transpiled to javascript. I'm having issues compiling because my model classes use these types, and thus transitively depend on Flutter.

Prequel to #13216

Related Issues

#13214

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.yaml and changelogs is not required.

  • I read the Contributor Guide and followed the process outlined there for submitting PRs.
  • My PR includes unit or integration tests for all changed/updated/fixed behaviors (See Contributor Guide).
  • All existing and new tests are passing.
  • I updated/added relevant documentation (doc comments with ///).
  • The analyzer (melos run analyze) does not report any problems on my PR.
  • I read and followed the Flutter Style Guide.
  • I signed the CLA.
  • I am willing to follow-up on review comments in a timely manner.

Breaking Change

Does your PR require plugin users to manually update their apps to accommodate your change?

  • Yes, this is a breaking change.
  • No, this is not a breaking change.

@Rexios80 Rexios80 changed the title Create a cloud_firestore_types package that can be used in dart-only contexts feat(cloud_firestore) Create a cloud_firestore_types package that can be used in dart-only contexts Aug 25, 2024
@Rexios80 Rexios80 changed the title feat(cloud_firestore) Create a cloud_firestore_types package that can be used in dart-only contexts feat(cloud_firestore): Create a cloud_firestore_types package that can be used in dart-only contexts Aug 25, 2024
@Rexios80
Copy link
Contributor Author

Meh I'm not sure this actually improves the state of things

@Rexios80 Rexios80 closed this Aug 25, 2024
@Rexios80
Copy link
Contributor Author

Actually I think having these available would help with json serialization

@Rexios80 Rexios80 reopened this Aug 25, 2024
@milyes
Copy link

milyes commented Aug 27, 2024

Description

Je travaille sur du code Dart qui est transposé en javascript. J'ai des problèmes de compilation car mes classes de modèle utilisent ces types et dépendent donc de manière transitive de Flutter.

Préquelle de #13216

Questions connexes

#13214

Liste de contrôle

Avant de créer cette demande de tirage, vérifiez qu'elle répond à toutes les exigences énumérées ci-dessous en cochant les cases correspondantes ( [x]). Cela garantira un processus de révision fluide et rapide. La mise à jour des journaux des pubspec.yamlmodifications et des modifications n'est pas requise.

  • J'ai lu le Guide du contributeur et suivi le processus qui y est décrit pour soumettre des PR.
  • Mon PR inclut des tests unitaires ou d'intégration pour tous les comportements modifiés/mis à jour/corrigés (voir le Guide du contributeur ).
  • Tous les tests existants et nouveaux sont réussis.
  • J'ai mis à jour/ajouté la documentation pertinente (commentaires de documentation avec ///).
  • L'analyseur ( melos run analyze) ne signale aucun problème sur mon PR.
  • J'ai lu et suivi le guide de style Flutter .
  • J'ai signé le CLA .
  • Je suis disposé à donner suite aux commentaires d'évaluation dans les meilleurs délais.

Changement radical

Votre PR exige-t-il que les utilisateurs de plugins mettent à jour manuellement leurs applications pour s'adapter à votre changement ?

  • Oui, c’est un changement radical.
  • Non, ce n’est pas un changement radical.

@Rexios80
Copy link
Contributor Author

I would like to include DocumentReference in this package as well since that can be in model classes, but that class seems to be more complex. Would making a base class maybe work?

@Lyokone
Copy link
Contributor

Lyokone commented Aug 27, 2024

Hello @Rexios80, I'm not sure it will be used by other people and thus might not be useful to include it directly in the main FlutterFire repository. Would using a fork directly on your side be easier?

@Rexios80
Copy link
Contributor Author

@Lyokone If cloud_firestore doesn't use the types from this package then it might as well not exist. I plan on using these in my own published packages, so it needs to be published as well. I'm working on firebase_js_interop which allows you to write Firebase Cloud Functions in Dart. I hope that would be a popular enough use-case to warrant this change. This isn't the first time I've run into this issue, but last time I found a workaround. Having a cloud_firestore_types package would allow me to remove the workaround package firebase_rules_convert.

I need to be able to create json converters from the cloud_firestore types to the js_interop types, but I can't do that since cloud_firestore depends on flutter.

@Rexios80
Copy link
Contributor Author

@Lyokone Do you need anything from me on this?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants