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/ssr-navigator #674

Merged
merged 7 commits into from
Jun 14, 2022
Merged

Fix/ssr-navigator #674

merged 7 commits into from
Jun 14, 2022

Conversation

JordaneNS
Copy link
Collaborator

Détails de la PR :

Lors du pull de la branche develop, 2 bugs sont apparus :

  • une erreur "navigator is not defined" apparaissait lors du premier rendu d'une page. Pour éviter cette erreur j'ai ajouté dans les affichages conditionnels typeof navigator !== "undefined"
  • une erreur de SSR est apparue sur la page de la randonnée (id: 501). L'erreur nous dit qu'il est impossible de serializer une valeur qui est undefined. Et effectivement sur cette rando il apparaît que le signage["703"].imageUrl renvoyé par l'api est undefined :
    image

L'erreur n'est pas présente sur la démo mais je pense qu'il est quand même important de la régler.
Pour se faire, je suis passée par la méthode SanitizeState déjà utilisée sur la page search.tsx afin d'éviter l'erreur de serialization.

Screenshots

  • Erreur du navigator
    image

  • Erreur SSR trek/[detailsId]
    image

@camillemonchicourt camillemonchicourt requested a review from dtrucs June 2, 2022 12:46
@dtrucs
Copy link
Collaborator

dtrucs commented Jun 2, 2022

Hello,
Bien vu pour l'erreur SSR de signage! 👍
Cela dit tu n'aurai pas dû t'embêter avec ça et ouvrir un ticket. En ayant implémenté cette partie, donc ce bug, j'aurai pris la responsabilité du fix.

Je pense que ton fix est peut-être un peu overkill, et ça contourne le prefetching de React-query.

Remplacer cette ligne
https://github.com/GeotrekCE/Geotrek-rando-v3/blob/main/frontend/src/modules/signage/adapter.ts#L9
par
imageUrl: rawSignage.attachments?.find(({ type }) => type === 'image')?.thumbnail ?? null,
(+ répercuter la modification sur le typage s/undefined/null) pour cette prop devrait faire le taf.

Je propose de retirer ton commit et tu me dis si tu (ou je) fais le fix ci-dessus.

Pour le reste de la PR, parfait 👍

@JordaneNS
Copy link
Collaborator Author

Hello @dtrucs !
Merci pour ton retour !
Je me suis occupée du fix : j'ai bien pris en compte les modifications et les erreurs sur le typage :)

frontend/src/components/Map/DetailsMap/PointsSignage.tsx Outdated Show resolved Hide resolved
frontend/src/components/Map/DetailsMap/PointsSignage.tsx Outdated Show resolved Hide resolved
frontend/src/modules/signage/interface.ts Outdated Show resolved Hide resolved
@JordaneNS
Copy link
Collaborator Author

@dtrucs ok it's done

@camillemonchicourt camillemonchicourt merged commit 3642367 into GeotrekCE:develop Jun 14, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants