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

[TECH] Permettre l'utilisation npm@^8.3.1 #4601

Merged
merged 2 commits into from
Jul 1, 2022

Conversation

nlepage
Copy link
Member

@nlepage nlepage commented Jun 30, 2022

🦄 Problème

Suite à la montée en node v16.14.0 / npm v8.x sur pix-ui, un bug de npm empêche npm install de fonctionner correctement sur pix-ui.

Le bug a été corrigé en npm v8.13.2.

La configuration des projets pix n'autorise que npm = v8.3.1.

Cela risque d'être gênant pour les personnes qui travaille sur pix-ui et sur les projets pix (devoir basculer entre npm v8.3.1 et npm v8.13.2).

🤖 Solution

Autoriser l'utilisation de npm >= v8.3.1 <= v8.13.2 sur les projets pix.

🌈 Remarques

Stabilisation des package-lock.json

Afin de stabiliser les package-lock.json (garantir que ces fichiers ne soient pas modifiés si on lance la commande npm install), la commande npm install a été exécutée en npm v8.3.1 et v8.13.2 sur chacun des projets.

Cela provoque un gros diff, notamment pour certains projets dont la version du package-lock.json a été mise à jour.

Conflit de peer dependency bookshelf -> knex

Bien que non documenté, il semble qu'un changement dans la vérification des peer dependencies ait été introduit entre la v8.3.1 et la v8.13.2 de npm.

Lors d'un npm install sur l'api en npm v8.13.2 on obtient l'erreur suivante :

While resolving: [email protected]
Found: [email protected]
node_modules/knex
  knex@"^2.1.0" from the root project
  knex@"2.1.0" from [email protected]
  node_modules/extract-pg-schema
    extract-pg-schema@"^3.1.7" from [email protected]
    node_modules/schemalint
      schemalint@"^0.5.10" from the root project

Could not resolve dependency:
peer knex@">=0.15.0 <0.22.0" from [email protected]
node_modules/bookshelf
  bookshelf@"^1.2.0" from the root project
  peer bookshelf@">= 1" from [email protected]
  node_modules/bookshelf-json-columns
    bookshelf-json-columns@"^3.0.0" from the root project

Conflicting peer dependency: [email protected]
node_modules/knex
  peer knex@">=0.15.0 <0.22.0" from [email protected]
  node_modules/bookshelf
    bookshelf@"^1.2.0" from the root project
    peer bookshelf@">= 1" from [email protected]
    node_modules/bookshelf-json-columns
      bookshelf-json-columns@"^3.0.0" from the root project

L'utilisation de [email protected] avec bookshelf ne semble pas avoir poser de problème jusque là, on peut donc "mute" l'erreur en utilisant overrides, voir api/package.json

💯 Pour tester

Faire fonctionner les différents projets avec npm v8.3.1 ou supérieur.

@nlepage nlepage added the cross-team Toutes les équipes de dev label Jun 30, 2022
@nlepage nlepage self-assigned this Jun 30, 2022
@pix-service
Copy link
Contributor

@nlepage nlepage force-pushed the tech-allow-npm-above-8.3.1 branch 3 times, most recently from 30afadc to d3ef68a Compare July 1, 2022 08:48
@nlepage nlepage marked this pull request as ready for review July 1, 2022 09:28
@nlepage
Copy link
Member Author

nlepage commented Jul 1, 2022

Il faut capper la version de npm à v8.13.2.

@nlepage
Copy link
Member Author

nlepage commented Jul 1, 2022

La seule application qui a des modifications conséquentes dans ses dépendances est certif, il faudrait donc faire une recette sur certif.

Copy link
Contributor

@MathieuGilet MathieuGilet left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Test fonctionnel de Pix Certif OK.

@pix-service-auto-merge pix-service-auto-merge merged commit 8fcc2ec into dev Jul 1, 2022
@pix-service-auto-merge pix-service-auto-merge deleted the tech-allow-npm-above-8.3.1 branch July 1, 2022 13:05
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
cross-team Toutes les équipes de dev Func Review OK PO validated functionally the PR 🚀 Ready to Merge Tech Review OK
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants