FazBrowse GitHub Viewer | Trending |
URL:
| Home
Tools: [Download Repo ZIP]   [Original HTTPS Page]

Use direct paths to react-icons in prod build by Hyperkid123 · Pull Request #3448 · patternfly/patternfly-react · GitHub

/ patternfly-react Public

Use direct paths to react-icons in prod build - #3448

Merged
evwilkin merged 1 commit into
patternfly:masterfrom
Hyperkid123:use-transform-plugin-on-icons
Jan 9, 2020
ConversationCommits (1)ChecksFiles changed
Merged

Use direct paths to react-icons in prod build#3448
evwilkin merged 1 commit into
patternfly:masterfrom
Hyperkid123:use-transform-plugin-on-icons

Conversation

Hyperkid123 commented Jan 2, 2020 •
edited
Loading

Copy link
Copy Markdown
Contributor

What: Addresses #3268

Added transform-imports for react-core to use direct paths to react-icons dependencies. This change marginally improves build sizes of libraries which depends on a library that is also using react-core package icons and then is consumed by some app.

Bundle size of an app using just PF4 Modal from a third-party library was reduced from 513kB to 27Kb

Right now, if you use any component with an icon from react-icons (Modal for example), it sometimes bundles all react-icons even if not used.

A reproducible example can be found here: https://github.com/Hyperkid123/pf-core-icons-deps

Detailed info can be found in #3268

Hyperkid123 mentioned this pull request Jan 2, 2020
Closed

patternfly-build commented Jan 2, 2020

Copy link
Copy Markdown
Collaborator

PatternFly-React preview: https://patternfly-react-pr-3448.surge.sh

karelhala previously approved these changes Jan 2, 2020
View reviewed changes

evwilkin commented Jan 6, 2020

Copy link
Copy Markdown
Member

@Hyperkid123 thanks for putting in this PR, it looks good to me but I am going to leave open for @redallen to review when he's back in office as well before merging.

Hyperkid123 force-pushed the use-transform-plugin-on-icons branch from 626a268 to 9d1ff7f Compare January 7, 2020 09:12
Hyperkid123 requested a review from karelhala January 7, 2020 09:19

codecov-io commented Jan 7, 2020 •
edited
Loading

Copy link
Copy Markdown

Codecov Report

Merging #3448 into master will increase coverage by <.01%.
The diff coverage is n/a.

@@            Coverage Diff            @@
##           master   #3448      +/-   ##
=========================================
+ Coverage   67.09%   67.1%   +<.01%     
=========================================
  Files         903     903              
  Lines       25471   25472       +1     
  Branches     2251    2251              
=========================================
+ Hits        17091   17092       +1     
  Misses       7342    7342              
  Partials     1038    1038
Flag Coverage Δ
#misc 95.45% <ø> (ø) ⬆️
#patternfly3 69.29% <ø> (ø) ⬆️
#patternfly4 64.2% <ø> (ø) ⬆️
Impacted Files Coverage Δ
...fly-3/patternfly-react/src/components/Tabs/Tabs.js 33.33% <0%> (+0.45%) ⬆️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update d88fae5...9d1ff7f. Read the comment docs.

evwilkin approved these changes Jan 9, 2020
View reviewed changes

evwilkin left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Choose a reason Spam Abuse Off Topic Outdated Duplicate Resolved Low Quality

LGTM

evwilkin merged commit 49814a7 into patternfly:master Jan 9, 2020

evwilkin commented Jan 9, 2020

Copy link
Copy Markdown
Member

addresses #3268

Hyperkid123 deleted the use-transform-plugin-on-icons branch January 9, 2020 13:57

Hyperkid123 commented Jan 9, 2020

Copy link
Copy Markdown
Contributor Author

@evwilkin I am doing more testing and this will have to be applied to all PF4 extensions using icons. I will do that now. I also have some updates regarding the react-core transform imports. I will post some notes and possible solutions to the issues thread.

evwilkin mentioned this pull request Jan 9, 2020
Closed

patternfly-build commented Jan 9, 2020

Copy link
Copy Markdown
Collaborator

Your changes have been released in:

  • @patternfly/react-catalog-view-extension@1.1.65
  • @patternfly/react-core@3.130.1
  • @patternfly/react-docs@4.16.78
  • @patternfly/react-inline-edit-extension@2.14.25
  • demo-app-ts@3.17.1
  • @patternfly/react-table@2.24.69
  • @patternfly/react-topology@2.11.53
  • @patternfly/react-virtualized-extension@1.3.66

Thanks for your contribution! 🎉

Hyperkid123 mentioned this pull request Jan 9, 2020
Closed

redallen commented Jan 13, 2020

Copy link
Copy Markdown
Contributor

This change is good for CJS bundles. If we are to ever switch to building react-core with Typescript we'll have to change all icon import paths manually to import directly from react-icons' dist.

redallen mentioned this pull request Jan 16, 2020
Merged
marshmalien mentioned this pull request Jan 31, 2020
Merged
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters. Learn more about bidirectional Unicode characters
Show hidden characters
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

evwilkin evwilkin approved these changes
redallen Awaiting requested review from redallen
jschuler Awaiting requested review from jschuler
+1 more reviewer
karelhala karelhala approved these changes
Reviewers whose approvals may not affect merge requirements

Assignees

No one assigned

Labels

Infrastructure

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

7 participants

Back | FazBrowse Home | New Git URL