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

🏷️ [Fix] Réparation de TOUS les types, et des erreur de lint #264

Merged
merged 105 commits into from
Oct 26, 2024

Conversation

camarm-dev
Copy link
Member

@camarm-dev camarm-dev commented Oct 5, 2024

🚀 Nouvelle Pull Request

Edit; Après moulte péripéties nous avons réussi avec @Kgeek33 à résoudre toutes les erreurs de lint et de types. On est entrain de tester cette assez grosse PR, n'hésitez à le faire aussi !

Comme j'ai pu le constater assez vite, les types de ce projet sont complètement cassés

Found 268 errors in 63 files.

Message de pnpm lint sur la branche main

J'ai donc résolu tout les erreurs de types, et j'ai pu ainsi faire fonctionner le linter, pour que le projet respecte les codes de styles définis dans le fichier de configuration.

De plus, j'ai ajouté un workflow qui permet de s'assurer que la base de code pass bien les tests du linter sur la branche main (on se rappelle du NextCourse.tsx cassé...) ! Cela permettra de garder mon travail de réparation des types utile dans le temps !

Informations importantes

J'aimerais que des tests assez profond soit effectués avant de merge ceci: j'ai touché à l'entièreté de la base de code et je ne suis pas sur que tout mes changements fonctionnent dans toutes les circonstances...

Liste des tests effectués

  • Test des devoirs avec Pronote
  • Test des notes avec Pronote
  • Test de l'EDT avec Pronote
  • Test des actualités avec Pronote (je ne peux pas tester)
  • Test de la cantine Turboself (je ne peux pas tester)
  • Test de la cantine ARD (je ne peux pas tester)
  • Tests ED
  • Tests skolengo

Checklist d'avant pull request

Veuillez cocher toutes les cases applicables en remplaçant [ ] par [x].

  • Vous avez testé de build le projet avec vos modifications et ce build a réussi
  • Vous respectez les conventions de codage et de nommage du projet
  • Vous utilisez la tabulation pour l'indentation afin de maintenir un code lisible
  • Cette pull request n'est pas un duplicata d'une autre
  • Cette pull request est prête à être revue (review) et fusionnée (merge)
  • Il n'y a pas de TODO (aka des annotations pour du code manquant) dans vos modifications
  • Il n'y a pas d'erreurs de langue dans votre code (grammaire, vocabulaire, conjugaison, orthographe)
  • Les détails des changements ont été décrits ci-dessous
  • Cette pull-request n'est pas une "breaking-change" (des modifications qui vont entraîner la modification du fonctionnement de certaines fonctionnalités déjà existantes)

Changelogs proposés

En gros le projet est bien clean.

Statut

+ 269/271 d'erreurs corrigées
- 2 erreurs restantes

(j'ai un peu perdu le fil, j'ai des erreurs qui se sont incrustées dans les node_modules maintenant...)

N'hésitez pas à tester la PR, ainsi qu'a ouvrir des conversations (nottament sur les bout de code que j'ai supprimé, car ils semblaient inutiles, compte tenu des types)

@Gabriel29306
Copy link
Contributor

Si tu fais une pr draft, tu pourrais juste rajouter un compteur ou un truc du genre dans le message initial pour que l'on sache où tu en es rendu ?

@camarm-dev camarm-dev changed the title [Fix] Réparation de TOUS les types, et des erreur de lint 🏷️ [Fix] Réparation de TOUS les types, et des erreur de lint Oct 5, 2024
@Kgeek33
Copy link
Contributor

Kgeek33 commented Oct 26, 2024

C'est tout ce que j'ai à dire pour l'instant.

De plus, j'ai remarqué la présence de pnpm-lock.yml dans le projet maintenant, vu que le projet utilise seulement npm actuellement je ne sais pas trop s'il faudrait switch vers pnpm dans l'état actuel...

Il faudrait supprimer package-lock.json si vous voulez faire le switch vers pnpm, mais je ne le recommande pas vraiment (car de souvenir il y avait des soucis avec react-native)

J'pense qu'il est préférable de privilégier npm plutôt sur pnpm
Après, c'est que mon avis

@Gabriel29306
Copy link
Contributor

npm est livré avec node donc plus simple pour la plupart des users

@camarm-dev
Copy link
Member Author

  • Bonne nouvelle j'ai résolu le bug qui empechait de faire quoique ce soit dans l'app (on pouvait pas changer de page enfaite)
  • Mauvaise nouvelle le require ne fonctionne pas (@Kgeek33):
 ERROR  Error: Element type is invalid: expected a string (for built-in components) or a class/function (for composite components) but got: object.

@Kgeek33
Copy link
Contributor

Kgeek33 commented Oct 26, 2024

  • Bonne nouvelle j'ai résolu le bug qui empechait de faire quoique ce soit dans l'app (on pouvait pas changer de page enfaite)
  • Mauvaise nouvelle le require ne fonctionne pas (@Kgeek33):
 ERROR  Error: Element type is invalid: expected a string (for built-in components) or a class/function (for composite components) but got: object.

Cool pour le changer de page, j'vais pouvoir retester

Pour le require, ça ne me surprend pas, j'ai fait un copié collé du typages dans le node_modules
J'vais voir pour une solution de contournement (pour l'instant, tu peux mettre de type any le temps que je trouve)

@camarm-dev
Copy link
Member Author

  • Bonne nouvelle j'ai résolu le bug qui empechait de faire quoique ce soit dans l'app (on pouvait pas changer de page enfaite)
  • Mauvaise nouvelle le require ne fonctionne pas (@Kgeek33):
 ERROR  Error: Element type is invalid: expected a string (for built-in components) or a class/function (for composite components) but got: object.

Cool pour le changer de page, j'vais pouvoir retester

Pour le require, ça ne me surprend pas, j'ai fait un copié collé du typages dans le node_modules J'vais voir pour une solution de contournement (pour l'instant, tu peux mettre de type any le temps que je trouve)

Le type est très bien ! C'est le fait de faire un require pour importer un composant que ne plait pas à React (il doit prendre ReanimtedGraph pour un objet quelconque...)

@camarm-dev
Copy link
Member Author

Oui c'est cela, React attend une fonction ou une classe mais ReanimatedGraph est interprété comme object (à cause du require très certainement)

@Kgeek33
Copy link
Contributor

Kgeek33 commented Oct 26, 2024

ahh j'ai ma 1ère erreur lors de la connexion
je regarde où

et je regarde aussi pour le require

@Gabriel29306
Copy link
Contributor

Peut-être que de mettre :

const ReanimatedGraph ...

On peut le remplacer par:

type ReanimatedGraph ...

(Je suis pas sûr, j'ai pas testé)

@camarm-dev
Copy link
Member Author

Peut-être que de mettre :

const ReanimatedGraph ...

On peut le remplacer par:

type ReanimatedGraph ...

(Je suis pas sûr, j'ai pas testé)

Ça fonctionne pas malheureusement... @Kgeek33 où tu as une erreur j'ai rien moi ?

Copy link
Contributor

@Kgeek33 Kgeek33 left a comment

Choose a reason for hiding this comment

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

ah mais j'ai crée moi même un bug sur une pr
voici la correction à faire

src/views/account/Home/Elements/HomeworksElement.tsx Outdated Show resolved Hide resolved
@Kgeek33
Copy link
Contributor

Kgeek33 commented Oct 26, 2024

Ça fonctionne pas malheureusement... @Kgeek33 où tu as une erreur j'ai rien moi ?

ça arrive 1 fois sur 2 ces erreurs là

@Kgeek33
Copy link
Contributor

Kgeek33 commented Oct 26, 2024

Alors pour le require...

Copy link
Contributor

@Kgeek33 Kgeek33 left a comment

Choose a reason for hiding this comment

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

Trouvé pour le require !

src/views/account/Grades/Graph/GradesAverage.tsx Outdated Show resolved Hide resolved
@camarm-dev
Copy link
Member Author

Et bien j'ai l'impression qu'on a terminé !

Copy link
Contributor

@Kgeek33 Kgeek33 left a comment

Choose a reason for hiding this comment

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

Tout est parfait 👌
Bon, juste les animations foireuses sur certaines pages (où ça provoque un écran noir de temps en temps) mais dans une autre PR, non ?

Testé depuis Pronote

@Kgeek33
Copy link
Contributor

Kgeek33 commented Oct 26, 2024

Mdrr jspas pq ça m'a mis le texte en gros 😂

@tom-theret
Copy link
Contributor

S’il vous plaît, merci de ne pas toucher au fichier UPHF, je sais qu'il y a 1 problème de types sur 1 ligne mais elle était inévitable quand j'ai créer cette implémentation.
Maintenant il y a peut-être des nouveaux truc dans Papillon, au quel cas je ferai ma PR de mon côté !

@Vexcited Vexcited merged commit 69e957d into PapillonApp:main Oct 26, 2024
1 check passed
@Kgeek33
Copy link
Contributor

Kgeek33 commented Oct 26, 2024

Gg @camarm-dev

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
✨ enhancement New feature or request
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants