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

Revoir l'architecture CSS #140

Closed
tbroyer opened this issue May 2, 2021 · 6 comments
Closed

Revoir l'architecture CSS #140

tbroyer opened this issue May 2, 2021 · 6 comments

Comments

@tbroyer
Copy link

tbroyer commented May 2, 2021

  • La global.scss à elle seule représente 222,6Ko des 321Ko du JavaScript (elle n'est pas minifiée dans le JS, donc beaucoup de \ pour escaping, la String JS fait 211,4K en length, soit 11K de \ ‼ et j'ai pas compté les espaces):

    ← JSON.stringify(w).length
    > 222603
    ← w.length
    > 211368
    ← JSON.stringify(w).length - w.length
    > 11235
    
  • La façon dont la global.scss est chargée dans les différents composants crée une nouvelle constructible stylesheet à chaque fois, avec parsing du CSS par le navigateur. Sur l'affichage initial de la page d'accueil, avec 3 composants (vmd-app, vmd-home et vmd-commune-or-departement-selector), le parsing de la global.scss (3 fois donc) compte pour environ un tiers du temps de traitement.
    image

  • Le fichier CSS chargé dans la page contient toutes les CSS de tous les composants, alors que ceux-ci ne servent à rien puisqu'ils sont "scopés" dans le shadow DOM (on notera que comme elle est minifiée, elle ne fait que 190Ko alors qu'elle contient bien plus de règles que la global.scss seule)

  • Il y a un assez gros layout shift quand les composants s'affichent enfin (vmd-app et vmd-home notamment, et c'est encore plus vrai sur un accès direct à une autre page qui n'affiche pas le texte de la page HTML).

À court terme, avec peu d'impact architectural, on peut traiter le point 2 en réutilisant une unique constructible stylesheet dans chaque composant. La global.scss sera toujours dupliquée entre le fichier CSS et le fichier JS, mais le navigateur ne parsera qu'une seule fois les règles CSS dans les composants et la réutilisera ensuite. Ça ne s'applique qu'aux navigateurs basés sur Chromium par contre, les autres (Firefox, Safari) ne supportant pas les constructible stylesheets de toutes façons (ils optimisent peut-être quand même la réutilisation du même cssText et ne souffrent peut-être pas du problème d'origine, je n'ai pas vérifié)

Pour éviter cette duplication entre fichier CSS et fichier JS, une possibilité serait d'inclure la global.scss (partagée entre la page HTML et tous les shadow DOMs) par URL via un <link rel="stylesheet">, mais sauf erreur ça signifierait gérer le processing Sass en dehors de Vite.

Pour traiter le troisième point, je me demande si ça ne vaudrait pas le coup de ne pas utiliser le Shadow DOM dans les composants; mais ça implique de gros changements, et notamment de ne plus pouvoir utiliser des slots par exemple (ou alors les injecter manuellement à l'initialisation des composants); la feuille de style inclurait les styles de composants mais ceux-ci seraient alors utilisés.
Ou alors ne pas utiliser Sass pour les styles des composants, ou gérer le processing Sass en dehors de Vite, et

Enfin pour le dernier point, il faudrait reprendre aussi l'architecture de l'application pour peut-être, par exemple, inclure une plus grande partie de vmd-app et vmd-home dans l'index.html (le logo et globalement la "barre de navigation" en haut de page; et pour vmd-home le fond du main-title et les home cards, les statistiques, etc. dans lequel pourraient être "injectés" le vmd-commune-or-departement-selector et les chiffres des statistiques.

Des changements architecturaux aussi conséquents ne valent peut-être pas le coup pour un site à la durée de vie limitée de toutes façons (même si quand même, trouver un moyen d'économiser la duplication de ~200Ko de CSS, sur un total de ~700Ko de JS+CSS, serait plus que pas mal !)

@tbroyer
Copy link
Author

tbroyer commented May 3, 2021

Une possible solution pour éviter que les styles des composants atterrissent dans le fichier CSS: https://github.com/e111077/vite-lit-element-ts-sass; par contre ça risque aussi de perturber l'extraction de CSS pour générer justement ce fichier CSS.
Voir aussi vitejs/vite#2522 qui empêche l'utilisation d'un <link rel=stylesheet> pour global.scss.

fcamblor added a commit that referenced this issue May 4, 2021
…centralize usages of reusable sass stylesheets
Floby pushed a commit that referenced this issue May 5, 2021
…centralize usages of reusable sass stylesheets
Floby pushed a commit that referenced this issue May 7, 2021
…centralize usages of reusable sass stylesheets
Floby pushed a commit that referenced this issue May 7, 2021
…centralize usages of reusable sass stylesheets
@joffreyBerrier
Copy link

Possible de revoir le front avec Tailwind ?

@fcamblor
Copy link
Collaborator

Non

@fcamblor
Copy link
Collaborator

fcamblor commented May 21, 2021

@tbroyer es-tu ok pour fermer l'issue, ta suggestion dans la PR #141 ayant été incorporée dans #137, je pense que les problèmes de performance que tu avais pinpointé sont résolus :-)

@fcamblor
Copy link
Collaborator

fcamblor commented May 21, 2021

@joffreyBerrier Je précise car j'ai été un peu sec : revoir toute l'archi CSS représenterait un gros investissement au niveau de l'énergie de l'équipe (et il faudrait maintenir toutes les PRs en cours sur ce gros refacto)

=> Pour moi ce n'est pas du tout d'actualité compte tenu du ROI que ça va représenter, je préfère investir l'énergie dans de nouvelles fonctionnalités plutôt que revoir en profondeur les choses

@fcamblor
Copy link
Collaborator

Je ferme l'issue, n'hésitez pas à la réouvrir si vous pensez que le sujet reste d'actualité :)

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

No branches or pull requests

3 participants