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

Add web.config for IIS deployment of the client #805

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

emmguyot
Copy link
Contributor

This merge request adds the necessary web.config file to deploy the client as static files in IIS (windows).

It also modifies the way the base URL is set in index.html.

Now 3 parameters are set for the client in .env :
REACT_APP_SERVER_BASE_URL : URL to access planka server API
PUBLIC_URL : URL to access planka client (i.e.: where the client is deployed and accessed)
BASE_URL : Base directory of the public URL with an ending /

@meltyshev
Copy link
Member

Hi! Thanks for your PR 🙏 I'll be sure to check it out as soon as I get back from vacation. Sorry for the delay.

@meltyshev
Copy link
Member

I'm trying to test, getting Cannot read properties of undefined (reading 'replace') when running npm run build in the client folder. Also getting Error: Cannot find module 'copy-webpack-plugin' when trying to build a docker image by docker build . -t planka in the root folder.

Also interested in compatibility with the current version as <%= BASE_URL %> is no longer inserted from the server, for example this is needed to be able to host Planka under the /planka path without having to rebuild the frontend.

@emmguyot
Copy link
Contributor Author

Currently studying why the build worked on my linux and not in my windows.

About docker, I haven't checked, I'll investigate that.

@emmguyot
Copy link
Contributor Author

Ok, my head is back ;-)
The error Cannot read properties of undefined (reading 'replace') you get, is because you haven't set the variable in the .env which are commented.
What do you suggest as a default value for these ?

@emmguyot
Copy link
Contributor Author

I've setted a few default values in client/.env
I've moved the copy-webpack-plugin requirement from dev to normal dependency in order the docker build works.

About your wondering for <%= BASE_URL %>, if you set the values in client/.env there shouldn't be any problem.

@meltyshev
Copy link
Member

Thanks for the fixes 🙏

About your wondering for <%= BASE_URL %>, if you set the values in client/.env there shouldn't be any problem.

I'm just worried that someone might already be using BASE_URL with a path (for example https://planka.app/path). In this case BASE_URL wouldn't be passed to index.html at runtime, meaning window.BASE_URL in index.html will always be the same as it was in client/.env when building the client. If someone needs to change the path from https://planka.app/path to https://planka.app/new-path, he will have to rebuild the client and specify a new path in client/.env, which is not very convenient. And in this case it's also impossible to use our Docker image.

Please correct me if I'm wrong.

@emmguyot
Copy link
Contributor Author

Ok, I see your goal.

First, you have to know that <%= BASE_URL %> doesn't work if your client is behind a IIS or an Apache server.
Anyway, I'll try a few things to tend towards your goal.

@emmguyot
Copy link
Contributor Author

I've managed to make the web.config path independent : It lightens the build process.

Though, I can't see a way to prevent to have the right folder in the index.html after the build.
Even the React documentation says to set the PUBLIC_URL environment variable.

To keep current installation running, I propose to make a new build:iis script in package.json. What do you think about that?

@meltyshev
Copy link
Member

I'm trying to figure out what web.config is, as far as I understand it's something like .htaccess for Apache. The build:iis option seems good, but I'm not sure if it's the client's responsibility to provide this file. Maybe it should just be in the documentation. But if you think it should be part of the client and it's more convenient that way, we'll definitely accept it.

@emmguyot
Copy link
Contributor Author

You're right that the web.config is a kind of .htaccess for IIS.
I think it's useful to provide it as it is required for the build directory to be deployed in IIS.
Although, it doesn't contain something that is really specific to planka, it is only specific to React. Anyway, I think it's useful for people that don't know how to deploy planka build directory in IIS.

In the same way, we may have to include .htaccess file.
I will include it if you mind.

@meltyshev
Copy link
Member

Sure, please include it :)

@emmguyot
Copy link
Contributor Author

I've added the specific build for IIS and Apache

If it's ok for you, I squash all commits

@nickbe
Copy link

nickbe commented Jul 26, 2024

I amd sorry, but I have to intervene at this point. Including things concerning Microsoft platform as part of the sourcecode - ESPECIALLY things concerning IIS - is a very bad idea. I'm working with microsoft servers for many years and if I learned one thing that is - there will be trouble because there always is - one day it will work - the next day it won't.

Some seperate documentation for people who want to work with IIS? Be my guest. But we will refuse plain and simple any kind of support for microsoft as a platform now and in the future - no matter in which version.

IIS is a bug riding on a deranged horse which thinks it's stable until it breaks down. Been there, done that.

@emmguyot
Copy link
Contributor Author

I'm afraid this is more a point of view than a factual problem with planka on IIS.

I regularly work with IIS on Windows and Apache on Linux, and I have to say that both works fine if it is correctly used and coded.

Anyway, I'm not here to force anyone for this PR.
I let the planka contributors judge for it.

@emmguyot emmguyot changed the title Add web.config for IIS deployement of the client Add web.config for IIS deployment of the client Sep 5, 2024
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.

3 participants