etesync / etesync-web

An EteSync web client
https://www.etesync.com
GNU Affero General Public License v3.0
246 stars 30 forks source link

Attempting to use the REACT_APP_DEFAULT_API_PATH doesnt work #185

Closed BeatLink closed 3 years ago

BeatLink commented 3 years ago

When I try to build the web UI, setting the above environment variable doesnt work. Here's the contents of my dockerfile

# Build the EteSync web client.

# Build Stage -----------------------------------------------------------------
FROM node:12 as build
ARG ETESYNC_WEB_TAG
ADD https://github.com/etesync/etesync-web/archive/${ETESYNC_WEB_TAG}.tar.gz etesync_web.tar.gz
RUN mkdir -p etesync-web \
 && tar xf etesync_web.tar.gz --strip-components=1 -C etesync-web \
 && cd etesync-web \
 && REACT_APP_DEFAULT_API_PATH="http://etebase-server:3735" yarn \
 && REACT_APP_DEFAULT_API_PATH="http://etebase-server:3735" yarn build

# Run Stage -------------------------------------------------------------------
FROM nginx
COPY --from=build /etesync-web/build /usr/share/nginx/html 

Any ideas?

tasn commented 3 years ago

Before I answer your actual question: this is not the API path you want, and you can't do it like this. The EteSync user's web browser is what's going to connect to the etesync server, which is not part of the swarm. You need the outside facing URL.

How do you know it's not working? Maybe try setting it to something like http://SOMETHING and then grep SOMETHING to check whether it works or not? I believe it should.

BeatLink commented 3 years ago

I did try to grep the url but it always goes to the api.etebase.com. Also this is a part of the docker compose image. http://etebase-server:3735 is the url to the internal docker container for the server. Wouldnt that be able to work?

tasn commented 3 years ago

I'm sure it used to work, and reading the react docs it should. Did you try doing what I suggested and then grepping for that string in the resulting JS file?

Also, yeah, even though you have a docker container it won't work. The reason is that the connecting is originating from the user's browser and not from the docker image.

BeatLink commented 3 years ago

I'm sure it used to work, and reading the react docs it should. Did you try doing what I suggested and then grepping for that string in the resulting JS file?

Yep, didnt work

BeatLink commented 3 years ago

My Dockerfile

# Build the EteSync web client.

# Build Stage -----------------------------------------------------------------
FROM node:12 as build
ENV GITHUB_TAG "v0.6.1"
ENV SERVER_URL "http://10.0.0.1:9000"
ADD https://github.com/etesync/etesync-web/archive/${GITHUB_TAG}.tar.gz etesync_web.tar.gz
RUN mkdir -p etesync-web \
 && tar xf etesync_web.tar.gz --strip-components=1 -C etesync-web \
 && cd etesync-web \
 && sed -i "s!https://api.etebase.com/partner/etesync/!${SERVER_URL}!g" ./src/constants/index.ts \
 && yarn \
 && yarn build

# Run Stage -------------------------------------------------------------------
FROM nginx
COPY --from=build /etesync-web/build /usr/share/nginx/html 
BeatLink commented 3 years ago

Build command

dockebuild . -f web-client.dockerfile

BeatLink commented 3 years ago

Grep command (inside container)

cat /usr/share/nginx/html/static/js/2.4682b6fb.chunk.js | grep http:10.0.0.1:9000

tasn commented 3 years ago

Let's try this on your computer rather than in docker. Are you able to get it to work then? I mean, your claim here is essentially equivalent to: "sed is broken" if you aren't enable to even just replace the URL.

BeatLink commented 3 years ago

Just tried it, still doesnt work. The file gets changed by sed but the final build doesn't contain my search keyword anywhere

tasn commented 3 years ago

I don't know what to tell you. It's just sed. It has nothing to do with React or EteSync. It sounds to me like you are maybe doing something wrong. If the file is changed by sed, and the string is not there after you sed (e.g. by grepping) it means that your sed command is wrong.

victor-rds commented 3 years ago

@tasn I can confirm that the REACT_APP_DEFAULT_API_PATH does nothing, to be more precise, the defaultServerUrl in src/constants/index.ts is never used anywhere.

I've changed the REACT_APP_DEFAULT_API_PATH, build, serve -s build, everything running as expected, but when I try to login without a server it points to https://api.etebase.com/api/v1/authentication/login_challenge/.

To test more I forked the code and set my api directly in src/constants/index.ts, still, same as before

victor-rds commented 3 years ago

Also the constant files we see:

export const defaultServerUrl = process.env.REACT_APP_DEFAULT_API_PATH ?? "https://api.etebase.com/partner/etesync/";

But this URL is not found on the generated javascript files, with exception of a debug map file

I'm reading the code now, I've an idea to change this url at runtime

BeatLink commented 3 years ago

Thank you! Now I know I'm not going crazy!

victor-rds commented 3 years ago

Thank you! Now I know I'm not going crazy!

I only know the basic of React, so there is large chance I'm wrong 😅

victor-rds commented 3 years ago

I was reading the code and yet couldn't find api.etebase.com anywhere, finally I found the problem, since the defaultServerUrl is not used anywhere the code fallback to the defaults on the etebase-js library.

BeatLink commented 3 years ago

ohh

BeatLink commented 3 years ago

Is it fixable?

tasn commented 3 years ago

Fixed. Nice catch @victor-rds!

BeatLink commented 3 years ago

Whoo! Ill try a build!

BeatLink commented 3 years ago

IT WORKS!

tasn commented 3 years ago

Great to hear, and sorry for the trouble!

BeatLink commented 3 years ago

No prob!

gkleen commented 1 year ago

Fixed. […]

@tasn can we get a release of this?