Define the `WASP_SERVER_URL` env var #1856

infomiho · 2024-03-05T17:54:28Z

Before we did the OAuth redirect like this:

[server] ---> [Google] ---> [client] --> (client sends the Google code to server)

We need the serverUrl variable for the new OAuth setup since we are doing the redirect like this:

[server] ---> [Google] --> [server (gets the code directly from the redirect)] ---> [client]

Martinsos

Looking good from a quick check!

The only thing I thought about is that the name doesn't fit the name we use for the client. So on client, to specify the server's URL, we use REACT_APP_API_URL, where API_URL is really the important part, REACT_APP is just prefix. And that prefix is also silly, it is just a reminder from using CRA.
So, if we call it "API_URL" there, it is a bit weird that we call it "SERVER_URL" here. Would be nicer if they are the same. If we don't want to do it now, maybe we can open an issue for it, or maybe it is better to handle now. I don't think "API_URL" is a good name really though. "SERVER_URL" sounds like a good name. So it would be breaking change. Maybe we can do it at the same time when we drop the REACT_APP prefix?

Btw pls also add to Changelog immediately what you did, once PR has taken final shape.

Martinsos · 2024-03-08T14:09:37Z

waspc/data/Generator/templates/sdk/wasp/server/config.ts

@@ -28,6 +28,7 @@ type CommonConfig = BaseConfig & {

 type EnvConfig = BaseConfig & {
  frontendUrl: string;
+  serverUrl: string;


Aha, so basically we need server to know its own URL! So far we had client know server URL, and we had server know client/frontend URL, but now we have server also knowing its own url.

sodic · 2024-03-08T14:46:57Z

waspc/data/Generator/templates/sdk/wasp/server/config.ts

@@ -28,6 +28,7 @@ type CommonConfig = BaseConfig & {

 type EnvConfig = BaseConfig & {
  frontendUrl: string;
+  serverUrl: string;


Does this include the port and the protocol?

It seems like it does. If so, we have a coupling between two config fields (port and serverUrl) and should modify our code to account for it.

So we need the server URL to be able to implement OAuth successfully. In dev, we need the default local server URL to be set automatically e.g. http://localhost:3001. If the user can change the port to 6000 then it would be good if we set the serverUrl to http://localhost:6000.

In the production use case, the port start being meaningless since they use a domain name e.g. if their port is 7000 for some reason, the server URL will still be the same e.g. https://my-server.com

This means, we need to make sure that. the default dev server is set according to the port, but in the production case we should just use the user env var.

sodic · 2024-03-08T14:50:57Z

waspc/src/Wasp/Generator/ServerGenerator/Common.hs

@@ -124,3 +125,6 @@ toESModulesImportPath :: FilePath -> FilePath
 toESModulesImportPath = changeExtensionTo "js"
  where
    changeExtensionTo ext = (++ '.' : ext) . fst . splitExtension
+
+defaultDevServerUrl :: String
+defaultDevServerUrl = "http://localhost:3001"


Do we want to support subpaths (like we do for the client)?

Since we don't yet support running the server from the subpath, I'd say no need to do that now. We'll cross that bridge when we get to it :)

sodic

Left a couple of questions I think we should address before merging.

Signed-off-by: Mihovil Ilakovac <mihovil@ilakovac.com>

infomiho · 2024-03-11T17:55:42Z

waspc/data/Generator/templates/react-app/src/config.js

@@ -1,9 +0,0 @@
-import { stripTrailingSlash } from "./universal/url";


This template file wasn't used anywhere, so I removed it. It's a leftover from migrating to the SDK.

infomiho · 2024-03-11T17:57:37Z

waspc/data/Generator/templates/sdk/wasp/server/config.ts

@@ -40,7 +41,7 @@ const config: {
  all: {
    env,
    isDevelopment: env === 'development',
-    port: parseInt(process.env.PORT) || 3001,
+    port: parseInt(process.env.PORT) || {= defaultServerPort =},


@sodic Now the default server port and default server URL are both defined in Haskell and the URL is constructed using the default port variable.

infomiho · 2024-03-11T18:00:06Z

@Martinsos Based on your comment, you are fine with the SERVER_URL name and would like to rename our client facing env variable to REACT_APP_SERVER_URL or similar. So, let's move this forward like this and worry about the client env in the future.

FYI We would still need to keep some sort of prefix. VITE_ is the default when using Vite (vs. REACT_APP_ that we kept from CRA).

Martinsos · 2024-03-12T10:13:54Z

@Martinsos Based on your comment, you are fine with the SERVER_URL name and would like to rename our client facing env variable to REACT_APP_SERVER_URL or similar. So, let's move this forward like this and worry about the client env in the future.

FYI We would still need to keep some sort of prefix. VITE_ is the default when using Vite (vs. REACT_APP_ that we kept from CRA).

Ideal would probably be something like WASP_, or CLIENT_ -> whatever makes most sense, but I would try to not have it specific to underlying technology.

I understand if we are not syncing the names now, but in that case pls create a GH issue to do this.

infomiho · 2024-03-13T08:25:17Z

I've created an issue to sync the env var names: #1885

infomiho changed the title ~~Define the WASP_SERVER_URL env var~~ Define the WASP_SERVER_URL env var Mar 5, 2024

infomiho mentioned this pull request Mar 5, 2024

Use Arctic for OAuth providers #1851

Merged

14 tasks

infomiho force-pushed the miho-server-url-config branch from 34801a6 to afd1622 Compare March 5, 2024 18:20

Martinsos reviewed Mar 8, 2024

View reviewed changes

sodic reviewed Mar 8, 2024

View reviewed changes

infomiho added 3 commits March 11, 2024 18:11

Define the WASP_SERVER_URL env var

f5e0f72

E2E tests

4ca3175

Signed-off-by: Mihovil Ilakovac <mihovil@ilakovac.com>

Fixes formatting

fb4115b

Signed-off-by: Mihovil Ilakovac <mihovil@ilakovac.com>

infomiho force-pushed the miho-server-url-config branch from afd1622 to fb4115b Compare March 11, 2024 17:11

infomiho added 3 commits March 11, 2024 18:50

Define default server port in Haskell

ca5d21e

Updates Changelog

8732466

Revert needless formatting

7d6bc3f

infomiho commented Mar 11, 2024

View reviewed changes

infomiho merged commit bad93f6 into arctic-feature Mar 13, 2024
6 checks passed

infomiho deleted the miho-server-url-config branch March 13, 2024 08:25

infomiho added a commit that referenced this pull request Mar 18, 2024

Define the WASP_SERVER_URL env var (#1856)

7329465

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Define the `WASP_SERVER_URL` env var #1856

Define the `WASP_SERVER_URL` env var #1856

infomiho commented Mar 5, 2024

Martinsos left a comment

Martinsos Mar 8, 2024

sodic Mar 8, 2024 •

edited

Loading

infomiho Mar 8, 2024

sodic Mar 8, 2024

infomiho Mar 8, 2024

sodic left a comment

infomiho Mar 11, 2024

infomiho Mar 11, 2024

infomiho commented Mar 11, 2024 •

edited

Loading

Martinsos commented Mar 12, 2024

infomiho commented Mar 13, 2024

		@@ -1,9 +0,0 @@
		import { stripTrailingSlash } from "./universal/url";

Define the WASP_SERVER_URL env var #1856

Define the WASP_SERVER_URL env var #1856

Conversation

infomiho commented Mar 5, 2024

Martinsos left a comment

Choose a reason for hiding this comment

Martinsos Mar 8, 2024

Choose a reason for hiding this comment

sodic Mar 8, 2024 • edited Loading

Choose a reason for hiding this comment

infomiho Mar 8, 2024

Choose a reason for hiding this comment

sodic Mar 8, 2024

Choose a reason for hiding this comment

infomiho Mar 8, 2024

Choose a reason for hiding this comment

sodic left a comment

Choose a reason for hiding this comment

infomiho Mar 11, 2024

Choose a reason for hiding this comment

infomiho Mar 11, 2024

Choose a reason for hiding this comment

infomiho commented Mar 11, 2024 • edited Loading

Martinsos commented Mar 12, 2024

infomiho commented Mar 13, 2024

Define the `WASP_SERVER_URL` env var #1856

Define the `WASP_SERVER_URL` env var #1856

sodic Mar 8, 2024 •

edited

Loading

infomiho commented Mar 11, 2024 •

edited

Loading