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: [OI-204] vite migration #446

Open
wants to merge 3 commits into
base: main
Choose a base branch
from
Open

Conversation

mmoio
Copy link
Collaborator

@mmoio mmoio commented Oct 18, 2024

  • Remove craco dependencies and configuration
  • adds vite dependencies and configuration
  • refactored envs to work with vite
    • Replaces REACT_APP with VITE as env prefixes
    • removes env-var and use vite import.meta

@mmoio mmoio requested a review from sebbalex October 18, 2024 08:36
@mmoio mmoio self-assigned this Oct 18, 2024
@mmoio mmoio force-pushed the feat/OI-204-craco-to-vite branch 2 times, most recently from 52209ee to e1d3beb Compare October 18, 2024 09:01
Copy link
Contributor

@sebbalex sebbalex left a comment

Choose a reason for hiding this comment

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

Env file should respect the short environment names. This is mostly due to CIs which use this mapping to run trough envs.
So, I suggest to rename back .env.production to .env.prod and .env.development to .env.dev

@mmoio
Copy link
Collaborator Author

mmoio commented Oct 22, 2024

Env file should respect the short environment names. This is mostly due to CIs which use this mapping to run trough envs. So, I suggest to rename back .env.production to .env.prod and .env.development to .env.dev

I'should ve explained this changes better in the pr description, but vite uses some defaults when loading env files, some documentation regarding this argument in this page https://vite.dev/guide/env-and-mode. One of this default behaviours is loading a .env.development in a dev environment, and .env.production when building.
I will need to make some changes regarding the uat environment as well creating a custom mode in the future,.

@mmoio mmoio force-pushed the feat/OI-204-craco-to-vite branch 2 times, most recently from 424b7c7 to 1c66cc8 Compare November 6, 2024 08:46
@mmoio mmoio temporarily deployed to prod/eu-central-1 November 6, 2024 08:46 — with GitHub Actions Inactive
@mmoio mmoio marked this pull request as ready for review November 6, 2024 08:54
@mmoio mmoio requested a review from a team as a code owner November 6, 2024 08:54
@mmoio
Copy link
Collaborator Author

mmoio commented Nov 6, 2024

Env file should respect the short environment names. This is mostly due to CIs which use this mapping to run trough envs. So, I suggest to rename back .env.production to .env.prod and .env.development to .env.dev

For simplicity sake, I've changed back the environment files back to the original names, so to keep the pipeline environments coherent with the other parts of the project.
The only remaining change is renaming .env.local to .env.development, since is the one loaded by default by vite during development. A file .env.local is instead always loaded by vite and the defined variables in this file are overridden only if there is a more specific env file is loaded

@mmoio mmoio requested a review from sebbalex November 6, 2024 08:58
@sebbalex
Copy link
Contributor

Env file should respect the short environment names. This is mostly due to CIs which use this mapping to run trough envs. So, I suggest to rename back .env.production to .env.prod and .env.development to .env.dev

For simplicity sake, I've changed back the environment files back to the original names, so to keep the pipeline environments coherent with the other parts of the project. The only remaining change is renaming .env.local to .env.development, since is the one loaded by default by vite during development. A file .env.local is instead always loaded by vite and the defined variables in this file are overridden only if there is a more specific env file is loaded

Thanks for the clarification, now I have a more comprehensive view.
We can keep .env.development for local environment, since it is a local development file it doesn't need to have specific naming.
We should fix .env.dev because of its conflicts inside.
And eventually, we can think to have a .env.local file to gather all the common variables among different environment in it.

Here you can find the deployment pipeline which will build and ship the frontend stuff.

This line will load the environment from matrix (which it can be one from "dev|uat|prod", line 17) and build the app:
export $(grep -v ^# .env.${{ github.event.inputs.environment }} | xargs) && yarn build

- changes .env.local to .env.development
- updates and adds commands to package.json
- updates .gitignore and fe deploy pipeline
- updates tsconfig
- updates vite configs
- updates index.tsx
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