Performance du pipeline scss_variables #13

Closed
opened 1 year ago by Eric · 5 comments
Eric commented 1 year ago
Owner

Le pipeline scss_variables est appelé à chaque compilation de fichier si j'ai bien compris. Donc si un seul fichier SCSS nécessite une injection de variables on recalcule à chaque fois la liste de variables à fournir y compris pour les fichiers qui ne s'en servent pas.

On peut améliorer le mécanisme avec une statique mais je me demandais si il ne serait pas possible de passer le fichier en cours de compilation afin de filtrer les traitements uniquement sur le ou les bons fichiers ?

Est-ce possible ou intéressant ?

Le pipeline `scss_variables` est appelé à chaque compilation de fichier si j'ai bien compris. Donc si un seul fichier SCSS nécessite une injection de variables on recalcule à chaque fois la liste de variables à fournir y compris pour les fichiers qui ne s'en servent pas. On peut améliorer le mécanisme avec une statique mais je me demandais si il ne serait pas possible de passer le fichier en cours de compilation afin de filtrer les traitements uniquement sur le ou les bons fichiers ? Est-ce possible ou intéressant ?
Collaborator

Bah la statique mangerait pas de pain pour sûr.

Je suis pas sûr de la pertinence de permettre de gérer sur quel fichier on les envoie.

Bah la statique mangerait pas de pain pour sûr. Je suis pas sûr de la pertinence de permettre de gérer sur quel fichier on les envoie.
Eric commented 1 year ago
Poster
Owner

Ben il me semble que ça serait pas mal, parce qu'on a pas besoin de toutes les variables à chaque fois. Après c'est sur que ça peut dépendre de la structuration des fichiers SCSS.

Ben il me semble que ça serait pas mal, parce qu'on a pas besoin de toutes les variables à chaque fois. Après c'est sur que ça peut dépendre de la structuration des fichiers SCSS.
Owner

La logique aurait été en effet de passer le nom du fichier qu'on compile au pipeline, à toute fin utile, mais ça n'a pas été fait lors de la création du pipeline donc on vit avec maintenant, et je sais pas si on veut vraiment introduite un nouveau pipeline pour ça... (vu que si on change le pipeline existant on casse la compat).

Cela dit l'intérêt n'aurait pas forcément été immense car tu as que le nom du fichier principal (genre "bootstrap.scss", "theme.scss") et ça te dit pas vraiment ce qu'il y a dedans, un utilisateur peut déplacer une inclusion de theme vers bootstrap parce qu'il a besoin de faire des @extend ou autre subtilité et paf ta compile va casser car tu as pas prévu ça.

Donc mon conseil c'est de pas trop jouer avec le feu et de faire quelque chose de simple et robuste.

Les variables sont a priori invariantes, mais cela dit rien ne nous le garantit, donc je pense que c'est au générateur des variables (ie celui qui remplit le pipeline), de mettre ses valeurs calculées en static si il a besoin pour éviter un calcul multiple (comme ça il les calcule 1 fois et il les ressort ensuite sans effort aux appels suivants).

La logique aurait été en effet de passer le nom du fichier qu'on compile au pipeline, à toute fin utile, mais ça n'a pas été fait lors de la création du pipeline donc on vit avec maintenant, et je sais pas si on veut vraiment introduite un nouveau pipeline pour ça... (vu que si on change le pipeline existant on casse la compat). Cela dit l'intérêt n'aurait pas forcément été immense car tu as que le nom du fichier principal (genre "bootstrap.scss", "theme.scss") et ça te dit pas vraiment ce qu'il y a dedans, un utilisateur peut déplacer une inclusion de theme vers bootstrap parce qu'il a besoin de faire des @extend ou autre subtilité et paf ta compile va casser car tu as pas prévu ça. Donc mon conseil c'est de pas trop jouer avec le feu et de faire quelque chose de simple et robuste. Les variables sont a priori invariantes, mais cela dit rien ne nous le garantit, donc je pense que c'est au générateur des variables (ie celui qui remplit le pipeline), de mettre ses valeurs calculées en static si il a besoin pour éviter un calcul multiple (comme ça il les calcule 1 fois et il les ressort ensuite sans effort aux appels suivants).
Eric commented 1 year ago
Poster
Owner

Ok, donc c'est fait avec les statiques pas de souci.
J'avais pas pensé au fait que si on ajoute des arguments ça casse la compatibilité car il faut indexer différemment les data.

Cela dit, on devrait peut-être toujours passer args même vide de façon à éviter ce genre de problème si un jour on veut en passer.

Je clos le ticket.

Ok, donc c'est fait avec les statiques pas de souci. J'avais pas pensé au fait que si on ajoute des arguments ça casse la compatibilité car il faut indexer différemment les data. Cela dit, on devrait peut-être toujours passer args même vide de façon à éviter ce genre de problème si un jour on veut en passer. Je clos le ticket.
Eric closed this issue 1 year ago
Owner

Voui +1 @Eric un exemple supplémentaire qui me fait dire aussi qu'on devrait changer la doc sur les pipelines en dépréciant le fait de passer directement un flux de données, et de pousser à ce qu'il y ait toujours un découpage args/data quelque soit les contenus. C'est le même principe que pour le ticket spip/spip#4972 où si yavait eu ce découpage depuis le début (avec args vide au départ donc), ça aurait permis "d'augmenter" le pipeline des années plus tard sans rien casser ou devoir inventer un second pipeline différent.

Voui +1 @Eric un exemple supplémentaire qui me fait dire aussi qu'on devrait changer la doc sur les pipelines en dépréciant le fait de passer directement un flux de données, et de pousser à ce qu'il y ait toujours un découpage args/data quelque soit les contenus. C'est le même principe que pour le ticket https://git.spip.net/spip/spip/issues/4972 où si yavait eu ce découpage depuis le début (avec args vide au départ donc), ça aurait permis "d'augmenter" le pipeline des années plus tard sans rien casser ou devoir inventer un second pipeline différent.
Sign in to join this conversation.
No Label
No Milestone
No Assignees
4 Participants
Notifications
Due Date

No due date set.

Dependencies

This issue currently doesn't have any dependencies.

Loading…
There is no content yet.