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

front: fix: local variable #60

Closed
wants to merge 1 commit into from
Closed

front: fix: local variable #60

wants to merge 1 commit into from

Conversation

ludovicdmt
Copy link
Member

Changement du nom de la variable de local dev pour que ça marche et être cohérent avec le back

@ludovicdmt ludovicdmt linked an issue Feb 18, 2025 that may be closed by this pull request
Copy link
Contributor

@Marc-AntoineA Marc-AntoineA left a comment

Choose a reason for hiding this comment

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

Incompréhension tout de même : pourquoi ça ne marche pas chez toi le code actuel alors que cela devrait ?!

import.meta.env.PROD est la manière officielle de distinguer la production pour un package Vite (obtenue en faisant un npm run build) et le développement (obtenu en faisant npm run dev).

Ajouter notre sauce par dessus me semble très piegeux. A minima extrêmement bien le commenter.


export const FULL_BASE_API_URL = IS_LOCAL_DEV
? "http://localhost:8000/api"
: import.meta.env.VITE_BASE_API_URL || `${window.location.origin}/api`
Copy link
Contributor

Choose a reason for hiding this comment

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

Cette variable d'environnement BASE_API_URL (le préfixe VITE est ajouté par Vite) est définie en production (via deploy/roles/frontend/templates/frontend.env). Elle est donc systématiquement présente.

Par cohérence, je suis pour n'avoir que l'option import.meta.env.VITE_BASE_URL ou bien la supprimer de frontend.env.

(et d'ajouter un commentaire dessus).

Mais du coup, si le code devient :

export const FULL_BASE_API_URL = IS_LOCAL_DEV
  ? "http://localhost:8000/api"
  : import.meta.env.VITE_BASE_API_URL

Alors il serait tout aussi correct d’écrire = ma proposition finale

export const FULL_BASE_API_URL =  import.meta.env.VITE_BASE_API_URL || "http://localhost:8000/api"

Copy link
Contributor

Choose a reason for hiding this comment

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

Il serait aussi + correct d’écrire : import.meta.env.BASE_URL que window.location.origin selon moi.

@ludovicdmt ludovicdmt closed this Feb 18, 2025
@ludovicdmt ludovicdmt deleted the fix-59/local-testing branch February 18, 2025 10:08
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.

ETQ dev je veux pouvoir tester en local et afficher les tuiles
2 participants