[DOC] Ecriture d'une ADR sur le choix d'arborescence pour les applications Front (PIX-9516) #7200
Conversation
machestla
force-pushed
the
pix-9516-write-adr-on-front-app-tree
branch
from
e0e05cc to
8ba2189
Compare
machestla
force-pushed
the
pix-9516-write-adr-on-front-app-tree
branch
from
8ba2189 to
b3db635
Compare
| - L'arborescence pods est bien supportée mais pas mise en avant par Ember. | ||
| Cela implique une duplication temporaire des helpers de test. (jusqu'à la fin de la migration) | ||
| - Tous les fichiers auront des noms similaires. Lors d'une recherche il faudra plus se reposer sur le nommage des répertoires. | ||
| - Les plugins d'IDE pour accéder aux tests associés aux fichiers ne fonctionnera plus avec le nouveau nommage (exemple : `Cmd+Maj+T` sur VSCode pour accéder aux tests liés) |
There was a problem hiding this comment.
Certains points peuvent être mitigés je pense :
Les plugins d'IDE pour accéder aux tests associés aux fichiers ne fonctionnera plus avec le nouveau nommage (exemple :
Cmd+Maj+Tsur VSCode pour accéder aux tests liés)
Dans l'arborescence "classic" sont dispersés donc les fichiers sont dur à trouver et le plugin ou shortcut est bienvenu. Avec "pods" tout est au même endroit, la plupart des IDEs centre dans la navigation le fichier où on se trouve donc on voit tous les tests associés à notre fichier à côté rendant le shortcut ou plugin moins indispensable.
Tous les fichiers auront des noms similaires. Lors d'une recherche il faudra plus se reposer sur le nommage des répertoires
Ici aucune différence avec avant nos fichiers dans le front s'apelle aussi tous pareil (ex: dashboard.hbs, dashboard.js, unit/dashboard_test.js, integration/dashboard_test.js, ...). On se servait déjà des dossiers pour les différencier. La différence est que là le nommage sera exactement le même peut importe le contexte avec des noms techniques (template, controller, route, ...). On inverse juste dans le path là où se fait le découpage technique (debut vs fin).
L'arborescence pods est bien supportée mais pas mise en avant par Ember.
Cela implique une duplication temporaire des helpers de test. (jusqu'à la fin de la migration)
J'en parle dans ce thread : #7200 (comment)
There was a problem hiding this comment.
@VincentHardouin j'ai présumé que ton "AH !" voulait dire "Ça fait beaucoup là !" 😅 (je suis peut être à côté de la plaque)
There was a problem hiding this comment.
Le Ah, c'est que ça me parait étonnant de privilégié une recherche à la main plutôt que automatique, mais chacun voit sa DX à sa porte :)
There was a problem hiding this comment.
On le privilégie pas, ici c'est les inconvénients de la solution. On a conscience que c'est une régression de DX par cet aspect là mais on estime qu'elle est mitigée par certains avantages de la solution et que dans la balance les pods nous apporte une meilleure DX au global 😄
machestla
force-pushed
the
pix-9516-write-adr-on-front-app-tree
branch
from
b3db635 to
a7372e1
Compare
machestla
force-pushed
the
pix-9516-write-adr-on-front-app-tree
branch
2 times, most recently
from
b889fbf to
fe5c394
Compare
|
Ca ressemble à #6252 ? |
La #6252 parle de mutualiser du code (ex: LangageSwitcher) entre les différentes applications (App, Orga, Certif, ...). Cette ADR parle de l'arborescence des applications front et la manière dont notre code est rangé. On pourrait plus dire que ça se rapproche de l'ADR sur l'API fait durant les tech days dans l'esprit : https://github.com/1024pix/pix/blob/dev/docs/adr/0051-nouvelle-arborescence-api.md |
|
|
||
| ### Inconvénients : | ||
|
|
||
| - L'arborescence pods est bien supportée mais pas mise en avant par Ember. |
There was a problem hiding this comment.
C'est bon signe ça ? Est-qu'on creuserai pas un peu dans les RFC Ember savoir si c'est pérenne ?
There was a problem hiding this comment.
Il y a une issue où justement il parle du support de pods et la conclusion c'est qu'ils maintiennent. Je retrouve le lien et je le poste ici 😄
There was a problem hiding this comment.
Il y a des discussions dans ces deux issues : emberjs/rfcs#650, emberjs/rfcs#651
Concrètement, ce que je retiens c'est que ils ne lâcheront pas le support sans une alternative viable par peur de réduire l'adoption de la version qui introduirait ce breaking change. Idéalement ils aimeraient que ça sorte du code de ember-cli pour être dans une lib à côté mais les échanges n'ont pas évolué depuis 3 ans et je n'ai pas entendu parler dans les conférences sur Polaris (v5).
Personnellement ça me suffit pour avoir confiance puisqu'ils ne droperont pas le support sans alternative viable.
There was a problem hiding this comment.
Je vais ajouter les deux liens dans l'ADR ^^
There was a problem hiding this comment.
@yannbertrand a trouvé un nouveau lien à ce sujet : emberjs/rfcs#906
There was a problem hiding this comment.
De ma lecture de cette issue emberjs/rfcs#906, rien de nouveau sous le soleil :
- Les pods intéressent particulièrement les grosses applications là où les plus petites restent plutôt dans la structure classic
- Pour des raisons, d'adoption avant de déprécié les pods il faut qu'une alternative existe qui incitera les grosses applications a continuer sur Ember
En résumé pas d'informations nouvelles par rapport aux liens listés ici à mon sens #7200 (comment)
There was a problem hiding this comment.
@yannbertrand a posé la question du futurs des pods Ember sur Twitter et a obtenu une réponse :
https://x.com/nullvoxpopuli/status/1711735226282475636
There was a problem hiding this comment.
☝️ Pour pas avoir à cliquer :
Question de Yann :
Is it planned to stop support for pods? What's the plan for Ember to avoid separation-by-type for big apps?
Réponse de NullVoxPopuli :
Gjs is the answer to everything.
Support for pods likely isn't going to be removed from ember-cli and embroider, but new features will replace the need for them.
Like how colorated components did for component pods.
Pour info Gjs, c'est expliqué ici sur notre Confluence : https://1024pix.atlassian.net/wiki/spaces/prescription/pages/3883696130/Lecture+RFC+first-class-component-templates
There was a problem hiding this comment.
Je note pour le moment :
Ajouter le lien emberjs/rfcs#906 à la PR
There was a problem hiding this comment.
Ça fait un peu flipper de se dire qu'on part sur une arborescence qui n'est pas vraiment officielle, et qui sera remplacé par autre chose un de ces jours...
Et en même temps, c'est quand même rassurant de voir qu'il n'y aura pas de dépréciation avant d'avoir une autre option. J'imagine que la direction sera « similaire ».
Ça signifie malgré tout qu'il faudra faire une autre migration sans doute. On peut espérer que l'esprit « clean archi » restera...
machestla
force-pushed
the
pix-9516-write-adr-on-front-app-tree
branch
from
fe5c394 to
04f6f61
Compare
machestla
force-pushed
the
pix-9516-write-adr-on-front-app-tree
branch
3 times, most recently
from
87daca0 to
70ab792
Compare
| - L'arborescence pods est bien supportée mais pas mise en avant par Ember. (cf. [emberjs/rfcs#650](https://github.com/emberjs/rfcs/issues/650), [emberjs/rfcs#651](https://github.com/emberjs/rfcs/issues/651)) | ||
| - Cela implique une duplication temporaire des helpers de test. (jusqu'à la fin de la migration) | ||
| - Tous les fichiers auront des noms similaires. Lors d'une recherche il faudra se reposer sur le nommage des répertoires et non sur plus sur celui des fichiers. | ||
| - Les plugins d'IDE pour accéder aux tests associés aux fichiers ne fonctionneront plus avec le nouveau nommage (exemple : `Cmd+Maj+T` sur VSCode pour accéder aux tests liés) |
There was a problem hiding this comment.
Après un peu d'expérimentation, j'en vois d'autres :
| - Les plugins d'IDE pour accéder aux tests associés aux fichiers ne fonctionneront plus avec le nouveau nommage (exemple : `Cmd+Maj+T` sur VSCode pour accéder aux tests liés) | |
| - Les plugins d'IDE pour accéder aux tests associés aux fichiers ne fonctionneront plus avec le nouveau nommage (exemple : `Cmd+Maj+T` sur VSCode pour accéder aux tests liés) | |
| - [La documentation](https://cli.emberjs.com/release/advanced-use/project-layouts/) est légère | |
| - Il y beaucoup de libertés sur la manière de construire son arborescence : structure par routes (ex : `app/pods/assessments/resume/route.js`), par entité métier (ex : `app/pods/assessment/model.js`), les tests au plus proche (`app/pods/**/*.integration_test.js`) ou en dehors de `app` (`tests/integration/**/*.test.js`), etc... On propose une phase d'expérimentation pour discuter de la structure à suivre plus tard. (// phrase à placer dans la décision ?) |
There was a problem hiding this comment.
Classic et pods peuvent cohabiter, on se retrouve avec 2 archi, nous sommes en cours d'expérimentation et peut-être qu'on en ressortira, plus tard, avec une arbo spécifique.
Il y a en effet, plus de liberté que d'utiliser l'arborescence classic
J'ajouterai, avec reformulation, ces points à l'ADR
Le dernier point est plutôt un avantage non ? @yannbertrand
| ### Inconvénients : | ||
|
|
||
| - L'arborescence pods est bien supportée mais pas mise en avant par Ember. (cf. [emberjs/rfcs#650](https://github.com/emberjs/rfcs/issues/650), [emberjs/rfcs#651](https://github.com/emberjs/rfcs/issues/651)) | ||
| - Cela implique une duplication temporaire des helpers de test. (jusqu'à la fin de la migration) |
There was a problem hiding this comment.
Attention à la confusion avec les helpers Ember. Peut être que utils/tests est plus adapté ?
There was a problem hiding this comment.
L'erreur qu'on a constaté est la suivante :
error Do not import a file from test-support into production code, only into test files ember/no-test-support-import
C'est similaire chez vous ?
There was a problem hiding this comment.
C'est une erreur de lint ?
On a une PR ici de test des pods qui est verte ✅ #6961
There was a problem hiding this comment.
En fait ça arrive à partir du moment où on a un import de Mirage donc pour des tests d'acceptance.
|
@machestla penser à mettre le label "cross-team" pour la prochaine fois |
|
Pour vous aider à vous projeter, voici une PR de migration de la page d'activité d'un prescrit du supérieur : #6961 |
| ## État | ||
|
|
||
| En cours |
There was a problem hiding this comment.
A ne pas oublier si ça passe
| ## État | |
| En cours | |
| ## État | |
| Accepté |
|
|
||
| ### Avantages | ||
|
|
||
| - Pas de refacto à réaliser |
There was a problem hiding this comment.
On pourrait indiquer que l'avantage est que c'est le modèle "de base" de Ember et que les guides sont par défaut dans ce modèle. Je pense aussi à notre ouvrage de référence à l'onboarding qui est Rock & Roll Ember
|
|
||
| En cours | ||
|
|
||
| ## Contexte |
There was a problem hiding this comment.
Ca serait cool que l'ADR donne un peu de documentation, de sources, de références.
A la lecture complète je me sens un peu que l'on me dit d'utiliser les pods mais je galère un peu à challenger les avantages / inconvénients, ce que ça apporte par rapport à l'orientation DDD que prend Pix, etc.
There was a problem hiding this comment.
Concrètement les pods nous permettent au lieu d'avoir une aborescence de fichiers directements découpé par typologie de fichiers (routes, models, controllers, components, ...) d'avoir une aborescence lié aux pages. On a donc un regroupement des fichiers liés au même endroit plutôt qu'éclaté dans plein de dossiers.
En ce sens, on peut se dire que ça se rapproche de ce qu'on veut faire côté API avec la nouvelle aborescence.
Est ce que c'est plus clair ? (je n'ai peut être pas bien compris la question 🤔)
There was a problem hiding this comment.
On m'a demande de reformuler ;
Disons que l'ADR s'intéresse pas trop au fond et aide pas le lecteur à comprendre toute la réflexion, le pourquoi, derrière la proposition des pods.
Je trouve que ça manque un peu de sources et liens pour me faire un avis sur un truc que l'on me demande de valider/challenger ; et que les éléments donnés me donne une impression de "crois moi sur parole" et de subjectivité.
Je pourrais reformuler simplement en disant : "La solution 2 semble la plus adéquate." -> pourquoi ?
Des exemples de trucs qui manquent
- de la documentation vers la définition du terme pods (parce que on dirait que c'est du vocab Ember le mot pods quand même).
- Que j'imagine aussi que l'on veut aussi alimenter l'avancé vers une archi + fonctionnelle, et que du coup le passage d'une arbo par type au pods ça va dans ce sens (regrouper les features, pas les éléments techniques).
- Des références sourcées à comment s'organisent les autres framework (qui pour la majorité sont justement non-opininated donc par défaut ils fonctionnement en rien du tout comme orga)
- Des screenshoot de la difference visuelle que ça donne entre solution 1 et 2
J'espère que ça aide à comprendre mon commentaire.
There was a problem hiding this comment.
C'est beaucoup plus clair, merci 👍
There was a problem hiding this comment.
Je note :
- ajouter le lien vers la doc de l'arbo pods Ember
- le deuxième point est répondu un peu plus tôt
- on peut ajouter des block-code avec la différences des deux arbo et ajouter un lien vers la PR test
machestla
force-pushed
the
pix-9516-write-adr-on-front-app-tree
branch
from
70ab792 to
017b21f
Compare
|
On a poussé un peu côté Modulix l'utilisation de pods. On a eu quelques déconvenues par rapport à nos attentes initiales. Ember recommande de conserver les tests dans un dossiers à part (
On a installé En résumé de nos essais on peut regrouper modèles, routes, templates, et composants (sans certitude de la meilleure manière de regrouper, comme évoqué plus haut), les styles (mais nécessite une dépendance qui a un impact louche), mais pas les tests. Notre branche est publiée sur #7242. |
Je trouve ça curieux que le CSS cassent les tests 🤔 Ça ressemble plutôt à un smell et quelque chose qu'il faudrait corriger idéalement. Bien vu pour les tests embarqués dans le bundle 👍 Peut être qu'on peut les exclure via une configuration ? La règle de lint evoquée ne semble en effet pas compatible avec le fait de mettre les tests au plus proche du code. On doit pouvoir la désactiver si elle ne nous semble pas pertinente ? |
| Création d'un dossier pods enfant du répertoire app avec la nouvelle arborescence. La PR avec cette structure se trouve [ici](https://github.com/1024pix/pix/pull/6961). | ||
| Les différentes équipes devront migrer pas à pas les fichiers vers la nouvelle arborescence. |
There was a problem hiding this comment.
Je pense qu'il y a d'autres contraintes comme revoir le bootcamp qu'on fait sur Ember, et peut-être de la doc ?
|
Nous avons fait un essai chez Pix 1d (#7232)
Il y a un petit hic : les tests sont embarqués dans le build et donc envoyé sur le navigateur. Il faudrait trouver le moyen de ne pas les inclure dans le build. |
frinyvonnick
force-pushed
the
pix-9516-write-adr-on-front-app-tree
branch
from
1e5a5d4 to
e8a3d86
Compare
|
Nous allons laisser les discussions non résolues ouvertes car elles font référence à des problématiques listées dans la décision et donc ça permet de les retrouver plus rapidement pendant la phase d'expérimentation. |
frinyvonnick
force-pushed
the
pix-9516-write-adr-on-front-app-tree
branch
from
e8a3d86 to
49aeeb6
Compare
pix-service-auto-merge
deleted the
pix-9516-write-adr-on-front-app-tree
branch
🦄 Problème
🤖 Proposition
Écrire un ADR
🌈 Remarques
Exemple de PR de migration de la page d'activité d'un prescrit du supérieur : #6961
💯 Pour tester