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

Unable to override basePath in next.config.js to allow running multiple Reflex apps on the same domain because of how Next.js serves /_next. #1633

Closed
nevdelap opened this issue Aug 20, 2023 · 9 comments · Fixed by #1724
Assignees
Labels
enhancement Anything you want improved feature request A feature you wanted added to reflex good first issue Good for newcomers

Comments

@nevdelap
Copy link
Contributor

nevdelap commented Aug 20, 2023

Describe the bug

  • reflex/utils/prerequisites.py can change compress to true or false based on next_compression in rxconfig.js .
  • As far as I can tell you cannot override anything else.
  • That next.config.js is in .web is in constants that as far as I can tell cannot be overridden.
  • If I reflex init, add basePath: "chat", to the .web/next.config.js that it created, and run reflex run it will run the site on a different path, e.g. /chat instead of /, in dev mode, including /chat/_next instead of /_next. This is good.
  • And if I do the same thing in a Dockerfile that runs reflex run --env prod it works too. This is good.

So what I want to suggest is the issue is that what I've done in those two last points is a workaround that with a couple of lines of code in prerequisites.py could be a supported feature.

Note: The reason this is needed is to change where Next.js serves /_next. The front-end and back-end can be served on whatever path you like on the domain (e.g. with Nginx proxy_pass), but it is /_next that is preventing running multiple sites on the same domain, as far as I've been able to figure out, because each app will have its own /_next that cannot both be served on that same path publicly.

(I'm new to Reflex and know nothing about Next.js other than what I've been investigating today, so maybe there's a better way that is already supported that I've not found.)

To Reproduce
As above.

Expected behavior
If prerequisites.py supported updating basePath like it updates compress based on rxconfig.js having next_compression = "true/false", by having next_base_path, changing the Next.js basePath would work out of the box to allow running multiple Reflex apps on the same domain with a one line addition to rxconfig.js in each app like next_base_path = "/chat", and next_base_path = "/someotherapp",

It would want to do one other thing. When it reports App running at: http://localhost:80 it would need to report App running at: http://localhost:80/chat. That might just happen since that message is coming from Next.js, but I don't think it did. (It's midnight as I write this and I need to go to bed. I'll update this when I've checked.)

Screenshots
N/A

Specifics (please complete the following information):

  • Python Version: 3.11
  • Reflex Version: 0.2.4
  • OS: Linux
  • Browser (Optional): Chrome

Additional context
N/A

Thanks. 🙏

@nevdelap nevdelap changed the title Unable to override basePath in next.config.js to allow running multiple reflex apps on the same domain. Unable to override basePath in next.config.js to allow running multiple reflex apps on the same domain because of how Next.js serves /_next. Aug 20, 2023
@nevdelap nevdelap changed the title Unable to override basePath in next.config.js to allow running multiple reflex apps on the same domain because of how Next.js serves /_next. Unable to override basePath in next.config.js to allow running multiple reflex apps on the same domain because of how Next.js serves /_next. Aug 20, 2023
@nevdelap nevdelap changed the title Unable to override basePath in next.config.js to allow running multiple reflex apps on the same domain because of how Next.js serves /_next. Unable to override basePath in next.config.js to allow running multiple reflex apps on the same domain because of how Next.js serves /_next. Aug 20, 2023
@nevdelap nevdelap changed the title Unable to override basePath in next.config.js to allow running multiple reflex apps on the same domain because of how Next.js serves /_next. Unable to override basePath in next.config.js to allow running multiple reflex apps on the same domain because of how Next.js serves /_next. Aug 20, 2023
@nevdelap nevdelap changed the title Unable to override basePath in next.config.js to allow running multiple reflex apps on the same domain because of how Next.js serves /_next. Unable to override basePath in next.config.js to allow running multiple Reflex apps on the same domain because of how Next.js serves /_next. Aug 20, 2023
@masenf masenf added enhancement Anything you want improved good first issue Good for newcomers feature request A feature you wanted added to reflex labels Aug 20, 2023
@masenf
Copy link
Collaborator

masenf commented Aug 20, 2023

Thanks for the detailed writeup, your request is very clear. The team has purposefully restricted exposing the next.config.js file in its entirety to keep things simpler for users and reduce our testing surface.

That said, this request seems quite reasonable, and not too hard to implement with the current code base. I think having a frontend_path config knob would be a good way to expose this setting without making it too next.js specific.

Is this something you'd be interested in contributing? I marked it as "good first issue" because it sounds relatively straightforward.

@nevdelap
Copy link
Contributor Author

nevdelap commented Aug 21, 2023

Thanks @masenf. I'm loving Reflex, so yes I'd like to. 👍

@masenf masenf assigned masenf and nevdelap and unassigned masenf Aug 21, 2023
@masenf
Copy link
Collaborator

masenf commented Aug 21, 2023

@nevdelap i assigned the issue to you. let me know if you need any assistance or advice.

nevdelap added a commit to nevdelap/reflex that referenced this issue Aug 22, 2023
- Tests.
- And sorted config in next.config.js template.
@nevdelap
Copy link
Contributor Author

nevdelap commented Aug 22, 2023

That commit is working locally and has tests. I've not deployed it side by side with another app with Nginx on a real domain yet, but I'm expecting it to work.

@masenf
Copy link
Collaborator

masenf commented Aug 22, 2023

I like the approach. Thanks for making the separate function, cleaning up the logic, and adding test cases; very nice.

@nevdelap
Copy link
Contributor Author

nevdelap commented Aug 23, 2023

I've deployed it on a real domain. Just had to change...

    location /chat/ {
        ...
        proxy_pass http://localhost:3001/;
        ...
    }

    location /_next/ {
        proxy_pass http://localhost:3001/_next/;
    }

to

    location /chat/ {
        ...
        proxy_pass http://localhost:3001/chat/;
        ...
    }

and it worked, then created a testapp with with just a reflex init, put backend_port, frontend_port, with different ports, and frontend_path="/testapp" into rxconfig.py, and duplicate the Nginx config with chat replaced with testapp and the different ports, and it worked, both sites running together on the same domain, one properly deployed in docker and whatnot, the other whacked together in 3 minutes in the terminal.

It still says App running on http://..... rather than http://...../chat, so maybe you want to assign me #1583 and I can see about fixing that at the same time and include the basePath.

@masenf
Copy link
Collaborator

masenf commented Aug 23, 2023

@nevdelap excellent, glad to see your progress. I have assigned #1583 to you as well, and they can come in together in the same PR if you wish.

@nevdelap
Copy link
Contributor Author

nevdelap commented Aug 28, 2023

Hi there. I've been using this for several apps running on my domain since then (they are all behind basicauth) and it is working perfectly. Are there any other things you can think of that I should test, or are there any places this should be mentioned in docos before I do a pull request? The only thing I've got in my list is to check that it can be overridden from the environment without adding anything extra.

Update: I checked overriding using FRONTEND_PATH env var. reflex run says Info: Overriding config value frontend_path with env var FRONTEND_PATH=/test, but it needs reflex init to have been run. If you reflex init first it works, but that reflex run doesn't work on its own despite that message is a bit misleading.

@nevdelap
Copy link
Contributor Author

I was thinking once it is out I'd post something in Discord #deployment.

masenf pushed a commit that referenced this issue Sep 2, 2023
…eflex apps off the same domain, and 1583 Show the correct info on where the site is being served. (#1724)

* Support setting Next.js basePath in Reflex config. (#1633)

- Tests.
- And sorted config in next.config.js template.

* Display the correct running at url with basePath if it is set. (#1583)

* Formatting, fixed by black.

* Fix indenting in test data.

* Fixed that conflict resolution shouldnt have included console.debug line.

* Rmove use of :=. Add http:// to url. Use urljoin to build url.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement Anything you want improved feature request A feature you wanted added to reflex good first issue Good for newcomers
Projects
None yet
2 participants