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

feat(vite): add HTTPS support with configurable SSL #8585

Merged

Conversation

AMoreaux
Copy link
Contributor

@AMoreaux AMoreaux commented Nov 19, 2024

The Pull Request adds support for local HTTPS configuration and introduces a new environment variable for the app's base URL.

  • Added new environment variable REACT_APP_BASE_URL in .env.example.
  • Introduced logic to utilize SSL certificates for local HTTPS in vite.config.ts.
  • Added validation to ensure SSL key and certificate paths are defined for HTTPS.
  • Included support for dynamic base URL setting based on the environment configuration.
  • Enhanced server configuration in vite.config.ts to handle HTTPS and local development.

Added HTTPS support by configuring SSL key and certificate paths. Updated environment example file to include a base URL and SSL configuration instructions.
Copy link
Contributor

@greptile-apps greptile-apps bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

PR Summary

This PR implements HTTPS support for the front-end development environment by adding SSL configuration options and updating the Vite server configuration.

  • Added SSL configuration variables (SSL_KEY_PATH, SSL_CERT_PATH) in /packages/twenty-front/.env.example
  • Updated /packages/twenty-front/vite.config.ts to handle HTTPS protocol and SSL file validation
  • Introduced REACT_APP_BASE_URL environment variable for configurable server URL with protocol support
  • Added file system checks to validate SSL certificate and key paths before enabling HTTPS
  • Implemented proper error handling for missing SSL files when HTTPS is requested

2 file(s) reviewed, 4 comment(s)
Edit PR Review Bot Settings | Greptile

packages/twenty-front/.env.example Outdated Show resolved Hide resolved
packages/twenty-front/.env.example Show resolved Hide resolved
packages/twenty-front/vite.config.ts Show resolved Hide resolved
packages/twenty-front/vite.config.ts Outdated Show resolved Hide resolved
Ensure the port is treated as a string for URL compatibility. This prevents potential issues when using numeric ports.

###### --------------> !!! FOR CHARLES AND FELIX !!! we can create a gist in twenty if you want <---------------------------
###### To configure a local certificate you can follow these step https://gist.github.com/AMoreaux/635ca9c38924d42a4d914dabe4376f72
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ideally we would add this to the developer documentation (mdx file) but I think it fits well with your gist like that for now. Could you just remove your comment so that we can merge?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We choose to change that in this PR / commit:
8cf6502

So now it should be good.

Removed instructions for local certification that were specific to Charles and Felix. This information is now outdated and no longer necessary for configuration.
Replaced REACT_APP_BASE_URL with VITE_ENABLE_SSL for better HTTPS handling. Simplified protocol and host configuration in the Vite server setup, removing the need for URL parsing and direct manipulation. Adjusted the example .env file to reflect these changes.
Copy link
Member

@FelixMalfait FelixMalfait left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Great! Thank you

@FelixMalfait FelixMalfait merged commit 3f98c2d into twentyhq:main Nov 21, 2024
11 checks passed
Copy link

Thanks @AMoreaux for your contribution!
This marks your 7th PR on the repo. You're top 5% of all our contributors 🎉
See contributor page - Share on LinkedIn - Share on Twitter

Contributions

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.

2 participants