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.

Sign up for GitHub

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

Jump to bottom

create vite plugin to treeshake uniconpaths.json #7275

Merged
merged 6 commits into from
Mar 26, 2024
Merged

create vite plugin to treeshake uniconpaths.json #7275

merged 6 commits into from
Mar 26, 2024

Conversation

Omkar76
Copy link
Contributor

@Omkar76 Omkar76 commented Feb 26, 2024 •
edited
Loading

Proposed Changes

  • Read through source code written in tsx and res files to find out names of icons being used.
  • Names are taken from string literals like "l-user-plus" or class names like "care-l-user-plus"
  • Remove unused icons from uniconpaths during transformation phase using a custom vite plugin
  • This brings size of file down to 20KB gzipped
  • Fixes Reduce uniconpaths.json bundle size #7270

@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

after treeshaking uniconpaths

@Omkar76 Omkar76 requested a review from a team as a code owner February 26, 2024 10:00
@vercel Vercel
Copy link

vercel bot commented Feb 26, 2024 •
edited
Loading

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 Feb 26, 2024 10:21am

@netlify Netlify
Copy link

netlify bot commented Feb 26, 2024 •
edited
Loading

✅ Deploy Preview for care-egov-staging ready!

Name Link
🔨 Latest commit 6140d48
🔍 Latest deploy log https://app.netlify.com/sites/care-egov-staging/deploys/65f9d5b7ca597600082afcf2
😎 Deploy Preview https://deploy-preview-7275--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 configuration.

@Omkar76
Copy link
Contributor Author

Omkar76 commented Feb 26, 2024

I wrote all plugin code in vite config itself. let me know if I should move it so separate file. (where? 😅 )

@github-actions GitHub Actions
Copy link

github-actions bot commented Mar 5, 2024

Hi, This pr has been automatically marked as stale because it has not had any recent activity. It will be automatically closed if no further activity occurs for 7 more days. Thank you for your contributions.

@github-actions github-actions bot added stale and removed stale labels Mar 5, 2024
rithviknishad
rithviknishad requested changes Mar 7, 2024
View reviewed changes
src/Components/Assets/AssetManage.tsx Outdated Show resolved Hide resolved
src/Components/Assets/AssetManage.tsx Outdated Show resolved Hide resolved
src/Components/Assets/AssetsList.tsx Outdated Show resolved Hide resolved
src/Components/Common/TemperatureFormField.tsx Outdated Show resolved Hide resolved
vite.config.mts Outdated Show resolved Hide resolved
@rithviknishad rithviknishad requested review from Ashesh3 and sainak March 7, 2024 13:55
Ashesh3
Ashesh3 approved these changes Mar 7, 2024
View reviewed changes
Copy link
Member

@Ashesh3 Ashesh3 left a comment

Choose a reason for hiding this comment

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

Apart from previous review, LGTM.

@Omkar76 Omkar76 force-pushed the treeshake-uniconpaths branch from 166e28b to 0bcc32b Compare March 17, 2024 09:50
@vercel Vercel
Copy link

vercel bot commented Mar 17, 2024

@Omkar76 is attempting to deploy a commit to the Open Healthcare Network Team on Vercel.

A member of the Team first needs to authorize it.

@Omkar76 Omkar76 requested a review from rithviknishad March 17, 2024 09:59
rithviknishad
rithviknishad reviewed Mar 17, 2024
View reviewed changes
Copy link
Member

@rithviknishad rithviknishad left a comment

Choose a reason for hiding this comment

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

LGTM, one last thing, forgot to mention earlier. Can we keep the plugin to a separate file? Maybe inside plugins directory?

Also, could you rename the plugin to be treeShakeCareIcons instead? Since the path json contains superset of unicon paths

@Omkar76 Omkar76 requested a review from rithviknishad March 17, 2024 16:37
@rithviknishad rithviknishad added needs testing Major Code Change This Tag represent PR, which will be causing major Code change in the platform and removed changes required labels Mar 18, 2024
@github-actions GitHub Actions
Copy link

👋 Hi, @Omkar76,
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.

@github-actions github-actions bot added the merge conflict pull requests with merge conflict label Mar 19, 2024
@nihal467
Copy link
Member

@Omkar76 clear the merge conflict

@Omkar76
Copy link
Contributor Author

Omkar76 commented Mar 22, 2024

Hi @nihal467,
I've resolved conflict but the label "merge conflict" is still there. Please manually remove it. Thanks.

@github-actions github-actions bot removed the merge conflict pull requests with merge conflict label Mar 22, 2024
@rithviknishad
Copy link
Member

@nihal467 can we get this tested ASAP as this would catch merge conflicts pretty quickly?

@nihal467
Copy link
Member

LGTM and cypress is also passing

@khavinshankar khavinshankar merged commit 2835d89 into ohcnetwork:develop Mar 26, 2024
32 of 35 checks passed
@github-actions GitHub Actions
Copy link

@Omkar76 Your efforts have helped advance digital healthcare and TeleICU systems. 🚀 Thank you for taking the time out to make CARE better. We hope you continue to innovate and contribute; your impact is immense! 🙌

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@khavinshankar khavinshankar khavinshankar approved these changes

@rithviknishad rithviknishad rithviknishad approved these changes

@Ashesh3 Ashesh3 Ashesh3 approved these changes

@sainak sainak Awaiting requested review from sainak

Assignees
No one assigned
Labels
Major Code Change This Tag represent PR, which will be causing major Code change in the platform needs review P2 tested
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

Reduce uniconpaths.json bundle size
5 participants
@Omkar76 @nihal467 @rithviknishad @Ashesh3 @khavinshankar