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: add classes containing path params and query params as const #1

Merged
merged 3 commits into from
Aug 30, 2024

Conversation

clementinedeb
Copy link
Collaborator

@clementinedeb clementinedeb commented Aug 28, 2024

Create a fork of GoRouterBuilder to add classes containing query param and path param as const to use in Epicery b2c app in this PR to match with GoRoute params map to send analytics event with those params' value.

Following this comment:

  • Branch fork/go-router-builder rebased on GoRouterBuilder latest release (commit 1520ffb) and reset to head to keep its name and serve as base for PR
  • New branch feat/add-const-for-query-path-params created with previous commit cherry-picked from fork/go-router-builder before reset

@clementinedeb
Copy link
Collaborator Author

@clementinedeb clementinedeb marked this pull request as ready for review August 29, 2024 07:18
@clementinedeb
Copy link
Collaborator Author

En réponse au commentaire:
Capture d’écran 2024-08-29 à 09 19 04

1 - Fonctions ajoutées dans le dernier commit 👍

2 - Pour ce point-ci, ClassQueryParams gérait uniquement les query params, un peu plus bas il y avait l'équivalent pour les path params, justement dû au formatage différent des params et pour les distinguer plus clairement à l'utilisation.

@clementinedeb clementinedeb requested a review from meynety August 29, 2024 07:24
Copy link

@meynety meynety left a comment

Choose a reason for hiding this comment

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

Erreurs au run du build_runner exécuté coté utilisation de la lib, avec la dep set pour la branche feat/add-const-for-query-path-params, dep qui s'est résolue au commit a27cade :

[SEVERE] go_router_builder on lib/modules/navigation/routes/scaffold_shell_routes.dart (cached):
An error `FormatterException` occurred while formatting the generated source for
  `package:flutter_b2c/modules/navigation/routes/scaffold_shell_routes.dart`
which was output to
  `lib/modules/navigation/routes/scaffold_shell_routes.go_router.g.part`.
This may indicate an issue in the generator, the input source code, or in the
source formatter.
Could not format because the source could not be parsed:

line 279, column 29 of .: Expected to find ';'.
    ╷
279 │   static const String tag = ''tag'';
    │                             ^^
    ╵

J'ai creusé la cause : route_config.dart:405 et route_config.dart:427 : escapeDartString fourni déjà des quotes autour de la String, elle se retrouvent donc en double dans la String générée aux lignes indiqués.

J'ai fais un petit code de test pour être sûr et éliminer toute suspicion possible de gestion de dépendance ou autres :

  • Depuis ce repo, sur le commit a27cade
import 'package:go_router_builder/src/route_config.dart';

void main() {
  String testQueryParam = "testQueryParam";
  String testPathParam = "testPathParam";
  String formatedQueryParam = GoRouteConfig.formatQueryParams(testQueryParam);
  String formatedPathParam = GoRouteConfig.formatPathParams(testPathParam);

  print(formatedQueryParam);
  print(formatedPathParam);

  String content =
      "static const String ${testQueryParam} = '${formatedQueryParam}';\n";
  print(content);
  content = "static const String ${testPathParam} = '${formatedPathParam}';\n";
  print(content);
}

Ce qui donne l'output :
image
En accord avec les erreurs ci-dessus au run du build_runner.

packages/go_router_builder/lib/src/route_config.dart Outdated Show resolved Hide resolved
packages/go_router_builder/lib/src/route_config.dart Outdated Show resolved Hide resolved
packages/go_router_builder/lib/src/route_config.dart Outdated Show resolved Hide resolved
@meynety
Copy link

meynety commented Aug 29, 2024

Est-ce que tu peux donner les droits "admin" à la team mobile sur ce repo s'il te plait ?

@clementinedeb clementinedeb requested a review from meynety August 29, 2024 09:44
@meynety meynety merged commit a844fb5 into fork/go-router-builder Aug 30, 2024
1 check passed
@meynety meynety deleted the feat/add-const-for-query-path-params branch August 30, 2024 09:25
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants