-
Notifications
You must be signed in to change notification settings - Fork 23
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
refactor(velo)!: use the @betagouv/aides-velo package #246
base: master
Are you sure you want to change the base?
Conversation
Salut Émile, effectivement je n'ai pas mis à jour les aides ces derniers mois, notamment parce que ma mission avec aides-jeune s'est interrompue et que je n'étais donc plus payé pour fournir un paquet à jour avec les aides. Peut être que l'approche la plus simple est que je te donne accès au dépôt ? |
Also, update miniatures generation by fetching images from github instead of using git submodules
src/lib/engine.ts
Outdated
@@ -1,18 +1,22 @@ | |||
import aidesVelo from '$lib/../aides.yaml'; | |||
import Engine, { type Situation } from 'publicodes'; | |||
import { rules as aidesVelo } from '@betagouv/aides-velo'; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Le fichier engine.js
envoyé au navigateur passe de 31.2kb à 62.3kb (compressés) avec cet PR. Ce n'est pas bien gênant mais je remarque que tout le code du paquet est envoyé au navigateur, y compris le manifeste des images et la class AideVeloEngine
qui ne sont pourtant pas utilisés par le client https://mesaidesvelo-eyorlr75e-novaways.vercel.app/_app/immutable/chunks/engine.3UuStsKr.js
Je pense que le code splitting ne fonctionne pas à cause du "barrel file" index.js
qui importe toute la librairie. Vu que l'import est fait côté client, il serait mieux de pouvoir importer uniquement les règles sans attraper tout le reste du code du paquet.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yes, je vais regarder comment optimiser tout ça
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
betagouv/publicodes-aides-velo#12 permet d'accéder uniquement aux submodules /data
ou /rules
si besoin. Cependant, ici vu que AidesVeloEngine
est utilisé pour résoudre les options d'une règle de type une possibilité
tout est nécessairement importer. Pour avoir uniquement les règles d'importer, il faudrait refaire cette fonction à la main. (A noter qu'il serait tout simplement plus judicieux à terme de l'avoir directement depuis publicodes
voir publicodes/publicodes#589).
@@ -1,6 +1,11 @@ | |||
{ | |||
"extends": "./.svelte-kit/mesaidesvelo.fr/tsconfig.json", |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
J'ai l'erreur suivante qui n'était pas là avant :
error during build:
TSConfckParseError: failed to resolve "extends":"./.svelte-kit/mesaidesvelo.fr/tsconfig.json" in /vercel/path0/tsconfig.json
at resolveExtends (file:///vercel/path0/node_modules/vite/dist/node/chunks/dep-R0I0XnyH.js:14139:8)
at parseExtends (file:///vercel/path0/node_modules/vite/dist/node/chunks/dep-R0I0XnyH.js:14076:24)
at parse$f (file:///vercel/path0/node_modules/vite/dist/node/chunks/dep-R0I0XnyH.js:13965:23)
at async loadTsconfigJsonForFile (file:///vercel/path0/node_modules/vite/dist/node/chunks/dep-R0I0XnyH.js:14788:24)
at async transformWithEsbuild (file:///vercel/path0/node_modules/vite/dist/node/chunks/dep-R0I0XnyH.js:14487:36)
at async Object.transform (file:///vercel/path0/node_modules/vite/dist/node/chunks/dep-R0I0XnyH.js:14612:32)
at async transform (file:///vercel/path0/node_modules/rollup/dist/es/shared/node-entry.js:17540:16)
at async ModuleLoader.addModuleSource (file:///vercel/path0/node_modules/rollup/dist/es/shared/node-entry.js:17757:36)
Error: Command "npm run build:retrofit" exited with 1
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
C'est étrange, je n'ai pas cette erreur en local.
Hello @mquandalle, je me permet de faire une PR afin d'utiliser le paquet
@betagouv/aides-velo
que j'ai créé à partir des modifications qui ont été faites par l'équipe de J'agis sur ton modèle des aides vélo. En effet, nous avons fait récemment une relecture et mise à jour de toutes les aides modélisées ainsi que rajouter certains paramètres et type de vélos (voir le CHANGELOG).Afin de faciliter la maintenance et la réutilisation du modèle par les autres services j'ai souhaité créer un paquet indépendant. Il me semblerait donc pertinent d'avoir une seule source de vérité pour le modèle et que mesaidesvelo.fr les utilises également.
Je commence dans cette PR le refacto afin que l'on puisse échanger à ce sujet, n'hésite pas à me ping si tu souhaites le faire de vive voix !