-
Notifications
You must be signed in to change notification settings - Fork 85
chore(design-system): define node engines and use it in setup node action #1542
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
base: main
Are you sure you want to change the base?
Conversation
e6d59f0 to
a033bb4
Compare
| - name: Setup npm version | ||
| if: ${{ env.NPM_VERSION }} | ||
| shell: bash | ||
| run: npm install -g npm@${{ env.NPM_VERSION }} |
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.
pourquoi supprimer cette parti car c'est le point essentiel pour que la step de check package lock fonction le fait d'avoir toujours la même version de npm pour tous, node étant plus secondaire.
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.
Car j'utilise la version npm que node apporte.
Le soucis qu'on a eu était qu'on est passé de node 22 à node 24 ce qui a provoqué des breaking change sur la nouvelle version majeure npm apportée.
Là comme on gère correctement la version de node, pas besoin de s'embêter avec la version npm selon moi.
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.
il faut fixer la bonne version de npm pour les user de volta du coup
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.
Ca c'est fait, j'ai passé la version volta en :
- node : 24.11.0
- npm : 11.6.1 (version par défaut livrée avec cette version de node)
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.
du coup au niveau engine cela veux dire qu'on l'on ne peux plus utiliser node 22.
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.
pas de range pour la version de npm, il faut fixer un numéro de version, car sinon on prend le risque d'avoir une version de fix de npm qui change la génération du package lock comme ce fu le cas entre la version 11.6.0 et 11.6.1
https://github.com/npm/cli/releases/tag/v11.6.1
npm/cli#8579
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.
je suis plutôt pour les ranges, au final, c'est peut-être l'outil de vérification du lock qui faudrait revoir. il faudrait regarder comment c'est fait sur d'autres projets Open Source mais figer une version de npm, je ne suis pas fan
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.
Ailleurs ils ne s'amusent à définir de version de npm tout court 😅
https://github.com/facebook/react/blob/main/packages/react/package.json#L49
https://github.com/react-hook-form/react-hook-form/blob/master/package.json#L137
https://github.com/mui/material-ui/blob/master/package.json#L178
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.
l'outil de vérification du lock ne fait rien de plus que prend un snap du package-lock d'exécuter la commande npm i [--ignore-script --package-lock-only et de comparer le package-lock avec le snap précédent.
S'il n'y a pas de diff alors le package.json et package-lock son synchro sinon il fail et indique de mettre à jours le package lock via un npm i.
sauf que celons la version de npm utiliser il peux y avoir des différences dans la génération du package-lock comme ce fu le cas entre la version 11.6.0 et 11.6.1 ou il au fix un truc qui a pour conséquence que package-lock générer en 11.6.0 et n'est plus le que même que si on le génère en 11.6.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.
Ailleurs ils ne s'amusent à définir de version de npm tout court 😅 https://github.com/facebook/react/blob/main/packages/react/package.json#L49 https://github.com/react-hook-form/react-hook-form/blob/master/package.json#L137 https://github.com/mui/material-ui/blob/master/package.json#L178
pour par que la clé engine sert à définir pour qu'elle version de node la lib et compatible, alors pour nos lib front cela na aucun intérêt mais pour des lib back la par contre sa change tous
|


This PR improves Node.js version management by making the CI automatically use the latest declared Node.js LTS from the
enginesfield inpackage.json, instead of relying on volta.It also removes npm version setup from the workflow for simplicity.
Main changes:
engines.nodeinpackage.json, always tracking the latest LTS.engines.nodeinpackage.jsonclearly lists supported Node.js versions.volta.nodeandvolta.npmare updated for local development consistency.