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

vite migration #5140

Merged
merged 27 commits into from
Apr 4, 2023
Merged

vite migration #5140

merged 27 commits into from
Apr 4, 2023

Conversation

sainak
Copy link
Member

@sainak sainak commented Mar 22, 2023

Proposed Changes

  • migrate build tools to vite
  • upgraded storybook and stories

TODO

  • verify that all components work as expected in a production build
  • fix service worker (checkout vite-pwa)
  • fix ci issues
  • fix storybook
  • fix generated css file size

@coronasafe/care-fe-code-reviewers @coronasafe/code-reviewers

Merge Checklist

  • Add specs that demonstrate bug / test a new feature.
  • Update product documentation.
  • Ensure that UI text is kept in I18n files.
  • Prep screenshot or demo video for changelog entry, and attach it to issue.
  • Request for Peer Reviews
  • Completion of QA

@sainak sainak requested a review from a team March 22, 2023 03:49
@sainak sainak requested a review from a team as a code owner March 22, 2023 03:49
@sainak sainak requested a review from mathew-alex March 22, 2023 03:49
@vercel
Copy link

vercel bot commented Mar 22, 2023

The latest updates on your projects. Learn more about Vercel for Git ↗︎

Name Status Preview Comments Updated (UTC)
care-storybook ✅ Ready (Inspect) Visit Preview 💬 Add feedback Apr 3, 2023 2:36pm

@netlify
Copy link

netlify bot commented Mar 22, 2023

Deploy Preview for care-egov-staging ready!

Name Link
🔨 Latest commit 137616f
🔍 Latest deploy log https://app.netlify.com/sites/care-egov-staging/deploys/642ae401f7155b000787fccf
😎 Deploy Preview https://deploy-preview-5140--care-egov-staging.netlify.app/
📱 Preview on mobile
Toggle QR Code...

QR Code

Use your smartphone camera to open QR code link.

To edit notification comments on pull requests, go to your Netlify site settings.

@sainak
Copy link
Member Author

sainak commented Mar 22, 2023

@rithviknishad the components contaning the phone number field break while previewing a production build, I'm not able to pin point the actual issue but here's the error that I got

Element type is invalid: expected a string (for built-in components) or a class/function (for composite components) but got: [missing argument].[missing argument]

image

please check this issue

Edit: fixed it

@sainak
Copy link
Member Author

sainak commented Mar 22, 2023

@rithviknishad the issue is fixed now.
We will have to be careful with what external libraries we use.

@gigincg
Copy link
Member

gigincg commented Mar 23, 2023

@sainak Can you update the netlify.toml and the Dockerfile so that @mathew-alex Can test this out in those environments?

cc: @rithviknishad

@sainak
Copy link
Member Author

sainak commented Mar 23, 2023

@gigincg changes to deployment config won't be needed as internally npm run build is calling vite
You can see the latest deploy preview, the components are working fine now but the css file is quite large as compared to cra builds, service worker might be broken, and I'm working on fixing the storybook

@shivankacker
Copy link
Member

shivankacker commented Mar 24, 2023

Just to quantify the impact of this PR, starting the current CRA app takes 1 minute and 36 seconds on my laptop with 16 gb ram and an i5-11400H @ 2.70GHz 2.69 GHz CP.
The Vite app takes just 40 seconds to start which makes the app start 215% faster than before. Hot reload also takes considerably less time than before. This allows CARE to be developed in less powerful computers. Great feat @sainak

@sainak
Copy link
Member Author

sainak commented Mar 24, 2023

@rithviknishad please update the ci for storybook the build commnad is build:storybook and the dist folder is build-storybook
also can you verify if the service worker is working correctly?

@rithviknishad rithviknishad removed the merge conflict pull requests with merge conflict label Mar 27, 2023
@sainak
Copy link
Member Author

sainak commented Mar 28, 2023

@gigincg @mathew-alex the PR is ready to be merged in develop with one breaking change that is the env variable PUBLIC_URL needs to be renamed to REACT_PUBLIC_URL

@github-actions github-actions bot added the merge conflict pull requests with merge conflict label Mar 29, 2023
@github-actions
Copy link

👋 Hi, @sainak,
Conflicts have been detected against the base branch. Please rebase your branch against the base branch.


This message is automatically generated by prince-chrismc/label-merge-conflicts-action so don't hesitate to report issues/improvements there.

@rithviknishad rithviknishad removed the merge conflict pull requests with merge conflict label Mar 29, 2023
Copy link
Member

@shivankacker shivankacker left a comment

Choose a reason for hiding this comment

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

LGTM!

@github-actions github-actions bot added the merge conflict pull requests with merge conflict label Apr 2, 2023
@github-actions
Copy link

github-actions bot commented Apr 2, 2023

👋 Hi, @sainak,
Conflicts have been detected against the base branch. Please rebase your branch against the base branch.


This message is automatically generated by prince-chrismc/label-merge-conflicts-action so don't hesitate to report issues/improvements there.

@khavinshankar
Copy link
Member

@sainak can you fix the merge conflicts

@sonarqubecloud
Copy link

sonarqubecloud bot commented Apr 3, 2023

SonarCloud Quality Gate failed.    Quality Gate failed

Bug A 0 Bugs
Vulnerability A 0 Vulnerabilities
Security Hotspot E 1 Security Hotspot
Code Smell A 1 Code Smell

No Coverage information No Coverage information
1.1% 1.1% Duplication

@khavinshankar khavinshankar added peer-reviewed and removed merge conflict pull requests with merge conflict labels Apr 4, 2023
@gigincg gigincg merged commit 89120b2 into ohcnetwork:develop Apr 4, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants