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

Consider removing lodash and moment packages #112

Open
hawkticehurst opened this issue Nov 18, 2021 · 4 comments
Open

Consider removing lodash and moment packages #112

hawkticehurst opened this issue Nov 18, 2021 · 4 comments
Labels
enhancement New feature or request good first issue Good for newcomers

Comments

@hawkticehurst
Copy link
Collaborator

hawkticehurst commented Nov 18, 2021

Feature Description

While reviewing another PR, I just noticed we're using lodash, which prompted me to do a check-up on our package weight and discovered that we're sitting at a pretty hefty 2MB after minification.

Of those 2MB, nearly 20% of that is caused by lodash and moment.

If you do a search for where these two packages are being used you'll find that:

Two functions from lodash are used in one file (although there will soon be a third function added to a second file when this PR gets merged).

One function from moment is also used in a file. Also as a side note, moment is currently installed as both a dependency and dev dependency 🤔.

In both of these cases, the functions we use from these libraries can be replaced with lighter alternative packages or even our own implementations of these utilities.

TLDR: I don't think 4 utility functions are worth a 20% increase in package weight.

Use Case

Package weight (and by proxy website weight) are an absolutely crucial metric for making sure the CDP website loads quickly.

Solution

As mentioned earlier we should remove lodash and moment and replace the 4 utility functions with a lighter utility package or our own custom implementation of the util functions.

@hawkticehurst hawkticehurst added the enhancement New feature or request label Nov 18, 2021
@hawkticehurst
Copy link
Collaborator Author

cc @tohuynh @BrianL3 @JacksonMaxfield

@tohuynh
Copy link
Collaborator

tohuynh commented Nov 18, 2021

I support the reason why we would remove these two packages.

Not sure about other use of lodash, but the one in my PR can be easily replaced with array.some().

Related to making our package lighter, we should remove semantic-ui-react as well and install only the components we need. The ones currently being used are Popup, Tab, and Loader. I've only used these because the styles of these components don't clash with Mozilla protocol.

If there's a UI component library that can be customized with Mozilla protocol styling and can be installed on a component basis, that would be great.

@hawkticehurst
Copy link
Collaborator Author

Oooh being able to remove semantic-ui would be fantastic too!

@BrianL3
Copy link
Collaborator

BrianL3 commented Dec 22, 2021

I support these changes. CDP Frontend, now in slimfit.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request good first issue Good for newcomers
Projects
None yet
Development

No branches or pull requests

4 participants