-
Notifications
You must be signed in to change notification settings - Fork 1
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
Issue 15 add local login page url #17
base: master
Are you sure you want to change the base?
Conversation
Test summaryRun details
View run in Cypress Dashboard ➡️ Failures
This comment has been generated by cypress-bot as a result of this project's GitHub integration settings. You can manage this integration in this project's settings in the Cypress Dashboard |
Test summaryRun details
View run in Cypress Dashboard ➡️ Failures
This comment has been generated by cypress-bot as a result of this project's GitHub integration settings. You can manage this integration in this project's settings in the Cypress Dashboard |
4ddcc91
to
5bb9c2e
Compare
5bb9c2e
to
583d9d3
Compare
.env.sample
Outdated
@@ -1 +1,2 @@ | |||
REACT_APP_API_URL='http://localhost:3000/api/v1/' | |||
REACT_APP_API_URL= |
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.
Le API contient le /api/v1
et le base non ? Si oui, on peut mettre ça quelque part ? (Genre readme ou dans le .sample
)
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.
Oui en effet, y'a de la duplication.
J'aime bien aussi :
REACT_APP_API_PATH='/api/v1'
REACT_APP_BACK_BASE_URL='http://localhost:3000'
Il y aurait juste const apiUrl = process.env.REACT_APP_API_URL!;
dans index.tsx à modifier
Pour cette ligne dans package.json par contre, j'ai l'impression qu'il y une coquille dans celui actuel :
"test:serve": "REACT_APP_API_URL=/api/v1 BROWSER=none yarn start",
C'est pas documenté dans le readme, pourquoi est-ce qu'on a cette commande yarn test:serve
tu sais
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.
chez moi, deploy/netlify plante.
J'arrive sur une page qui semble requetter en boucle.
5d6461b
to
db30b70
Compare
Salut @gael-boyenval |
.env.sample
Outdated
@@ -1 +1,2 @@ | |||
REACT_APP_API_URL='http://localhost:3000/api/v1/' | |||
REACT_APP_API_PATH= | |||
REACT_APP_BACK_BASE_URL= |
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 dirais qu'on peut remplacer BACK par API, pour indiquer que c'est la même application. Ça permet de moins se poser de questions.
@@ -14,7 +14,10 @@ import * as serviceWorker from "./serviceWorker"; | |||
import { Theme } from "./ui/Theme/Theme"; | |||
|
|||
const authToken = getOauthTokenOrRedirect(); | |||
const apiUrl = process.env.REACT_APP_API_URL!; | |||
|
|||
const apiPath = process.env.REACT_APP_API_PATH!; |
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 sûr que ce soit nécessaire de variabiliser le path de l'API. Ce n'est pas comme si on avait plusieurs versions de l'API pour l'instant.
const apiPath = process.env.REACT_APP_API_PATH!; | |
const apiPath = "/api/v1"; |
Hello ! J'ai fait une modif côté back pour que l'API soit sur le port 3001 en local afin qu'il n'y ait pas de redirections en boucle. J'ai réussi à faire fonctionner cette branche (pas de redirections) côté front en renseignant cela dans
|
e27e881
to
21a8495
Compare
21a8495
to
1a0e4cd
Compare
No description provided.