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

[dashboard] Optimize build #3830

Merged
merged 1 commit into from
Apr 22, 2021
Merged

[dashboard] Optimize build #3830

merged 1 commit into from
Apr 22, 2021

Conversation

aledbf
Copy link
Member

@aledbf aledbf commented Apr 8, 2021 •
edited
Loading

Changes:

Screenshot of network differences (main.*.chunk.css): -450Kb
Screenshot from 2021-04-08 06-25-20
Screenshot from 2021-04-08 06-25-36

@aledbf aledbf force-pushed the aledbf/dashboard branch 4 times, most recently from 0a48905 to b34686c Compare April 8, 2021 10:03
@aledbf
Copy link
Member Author

aledbf commented Apr 8, 2021 •
edited by werft-gitpod-dev-com bot
Loading

/werft run

👍 started the job as gitpod-build-aledbf-dashboard.13

@aledbf aledbf marked this pull request as ready for review April 8, 2021 10:31
This was referenced Apr 8, 2021
Closed
Closed
@aledbf aledbf force-pushed the aledbf/dashboard branch from b34686c to 2db110a Compare April 9, 2021 13:56
@theoludwig
Copy link

Also, we could use the new "Just In Time" mode from tailwind.
See: https://tailwindcss.com/docs/just-in-time-mode

@aledbf
Copy link
Member Author

aledbf commented Apr 19, 2021

Also, we could use the new "Just In Time" mode from tailwind.

I would prefer to delay that for a different PR. We only need to add mode: 'jit' once this PR is merged.

@aledbf aledbf force-pushed the aledbf/dashboard branch from 2db110a to 8df0b49 Compare April 19, 2021 19:03
@AlexTugarev
Copy link
Member

AlexTugarev commented Apr 20, 2021 •
edited by werft-gitpod-dev-com bot
Loading

/werft run

👍 started the job as gitpod-build-aledbf-dashboard.16

@AlexTugarev
Copy link
Member

AlexTugarev commented Apr 20, 2021 •
edited
Loading

Build is failing :-(
@aledbf, would you mind to rebase onto origin/main? It's possible that there were some related changes.

I'm curious to see that in action. If there are no differences, that'd be awesome!
Is purge applied automatically, or how to explain the difference of the css produced? And if true, we'd need to check the *.tsx to ensure, there are no concatenations of class names 🙈

UPDATE:

Tailwind is not purging unused styles because no template paths have been provided.

OK

@AlexTugarev
Copy link
Member

AlexTugarev commented Apr 20, 2021 •
edited by werft-gitpod-dev-com bot
Loading

From the build output I cannot see how this impacts the size of the css chunk.

File sizes after gzip:

  543.31 KB  build/static/css/main.b4639cee.chunk.css

Looks similar to previous state.

OTHO I ❤️ the eslint running on the code!

@aledbf aledbf force-pushed the aledbf/dashboard branch from 8df0b49 to aaad340 Compare April 20, 2021 11:38
@aledbf
Copy link
Member Author

aledbf commented Apr 20, 2021

Is purge applied automatically, or how to explain the difference of the css produced?

Yes, that what craco enables https://purgecss.com/guides/react.html#use-craco

@aledbf aledbf requested a review from AlexTugarev April 20, 2021 14:23
@AlexTugarev
Copy link
Member

AlexTugarev commented Apr 21, 2021 •
edited by werft-gitpod-dev-com bot
Loading

/werft run

👍 started the job as gitpod-build-aledbf-dashboard.18

@AlexTugarev
Copy link
Member

AlexTugarev commented Apr 21, 2021 •
edited
Loading

Yes, that what craco enables https://purgecss.com/guides/react.html#use-craco

I though it would not be enabled, because of the missing params. Let me check again...

@aledbf, I double-checked, it's not applied automatically. It's currently transferring ~620KB gzipped, while the original size is still ~8megs.

warn - Tailwind is not purging unused styles because no template paths have been provided.

We need to provide a template path like *.tsx in order to enable it, but also ensure that there is no class name concatenation in those files (otherwise too much will be purged)

@aledbf
Copy link
Member Author

aledbf commented Apr 21, 2021

I double-checked, it's not applied automatically. It's currently transferring ~620KB gzipped, while the original size is still ~8megs.

My fault. When I rebased against main I forgot the purge configuration in tailwind.config.js.

@AlexTugarev
Copy link
Member

AlexTugarev commented Apr 21, 2021 •
edited by werft-gitpod-dev-com bot
Loading

Awesome! The number of transferred kilobyte dropped to ~8 🎉

Just quickly scanned what might break according to the docs, i.e. concatenation of class names. But it appears to works 🤷🏻‍♂️, at least the Always Ready-To-Code icon size on the Login page seems to be the expected one.

https://github.com/gitpod-io/gitpod/blob/main/components/dashboard/src/Login.tsx#L27
https://github.com/gitpod-io/gitpod/blob/main/components/dashboard/src/admin/UserDetail.tsx#L192

AlexTugarev
AlexTugarev approved these changes Apr 21, 2021
View reviewed changes
Copy link
Member

@AlexTugarev AlexTugarev left a comment

Choose a reason for hiding this comment

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

Thanks @aledbf! I like the changes!
See another nit regarding the eslint versions.

@aledbf aledbf force-pushed the aledbf/dashboard branch from d963d13 to 9f7bb81 Compare April 21, 2021 10:09
@aledbf
Copy link
Member Author

aledbf commented Apr 21, 2021 •
edited by werft-gitpod-dev-com bot
Loading

/werft run

👍 started the job as gitpod-build-aledbf-dashboard.21

@aledbf aledbf force-pushed the aledbf/dashboard branch from 9f7bb81 to e88ec89 Compare April 21, 2021 10:15
@aledbf
Copy link
Member Author

aledbf commented Apr 21, 2021 •
edited by werft-gitpod-dev-com bot
Loading

/werft run

👍 started the job as gitpod-build-aledbf-dashboard.23

@aledbf
Copy link
Member Author

aledbf commented Apr 21, 2021 •
edited by werft-gitpod-dev-com bot
Loading

/werft run

👍 started the job as gitpod-build-aledbf-dashboard.24

@aledbf
Copy link
Member Author

aledbf commented Apr 21, 2021 •
edited by werft-gitpod-dev-com bot
Loading

/werft run

👍 started the job as gitpod-build-aledbf-dashboard.25

@aledbf
Copy link
Member Author

aledbf commented Apr 21, 2021 •
edited by werft-gitpod-dev-com bot
Loading

/werft run

👍 started the job as gitpod-build-aledbf-dashboard.26

@csweichel
Copy link
Contributor

csweichel commented Apr 22, 2021 •
edited by werft-gitpod-dev-com bot
Loading

/werft run

👍 started the job as gitpod-build-aledbf-dashboard.27

@aledbf
Copy link
Member Author

aledbf commented Apr 22, 2021 •
edited by werft-gitpod-dev-com bot
Loading

/werft run

👍 started the job as gitpod-build-aledbf-dashboard.28

@aledbf
Copy link
Member Author

aledbf commented Apr 22, 2021 •
edited by werft-gitpod-dev-com bot
Loading

/werft run

👍 started the job as gitpod-build-aledbf-dashboard.29

@aledbf aledbf merged commit c71b2d7 into main Apr 22, 2021
@aledbf aledbf deleted the aledbf/dashboard branch April 22, 2021 13:12
@jankeromnes jankeromnes added the changelog worth adding to www.gitpod.io/changelog label Apr 22, 2021
@jankeromnes jankeromnes added this to the April 2021 milestone Apr 22, 2021
@jankeromnes jankeromnes mentioned this pull request May 10, 2021
Closed
@gtsiolis gtsiolis mentioned this pull request May 17, 2021
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@AlexTugarev AlexTugarev AlexTugarev approved these changes

@svenefftinge svenefftinge Awaiting requested review from svenefftinge

@jankeromnes jankeromnes Awaiting requested review from jankeromnes

Assignees
No one assigned
Labels
changelog worth adding to www.gitpod.io/changelog component: dashboard
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
6 participants
@aledbf @theoludwig @AlexTugarev @csweichel @gtsiolis @jankeromnes