Skip to content
This repository has been archived by the owner on Feb 23, 2024. It is now read-only.

Evaluate if we can move away from Notice component #2280

Closed
senadir opened this issue Apr 23, 2020 · 6 comments · Fixed by #2664
Closed

Evaluate if we can move away from Notice component #2280

senadir opened this issue Apr 23, 2020 · 6 comments · Fixed by #2664
Assignees
Labels
focus: performance The issue/PR is related to performance. status: blocker Used on issues or pulls that block work from being released.
Milestone

Comments

@senadir
Copy link
Member

senadir commented Apr 23, 2020

Describe the bug
Investigating the increased bundle size in All Product it seems it was caused by us pulling in Notice component, that pulled in other dependencies like Dashicon, this could be fixed by #2140 or by building our custom component.

Notice in itself also pulls dashicons and some other dependencies.

@senadir senadir added the focus: performance The issue/PR is related to performance. label Apr 23, 2020
@senadir
Copy link
Member Author

senadir commented May 4, 2020

@nerrad I would love if you can help me evaluate this and reach a decision here, notices is hitting us hard now (the bulk of the weight introduced in #2399 is by notices) all products is the same, we can assume that notices will be added to every interactive block we will develop (all products, Cart and Checkout, single product and any future product).

We should evaluate if we should move it to an independent bundle or build something our own, the notice implementation is simple and straightforward, the snackbar is slightly more complicated but still very doable.

@nerrad
Copy link
Contributor

nerrad commented May 4, 2020

With the bulk of the weight coming from Dashicons, I'd prefer to do what's needed to eliminate that instead of building our own replacement for what we're using from GB core.

@nerrad nerrad mentioned this issue May 25, 2020
17 tasks
@nerrad nerrad added this to the 2.7.0 milestone Jun 2, 2020
@nerrad nerrad added the status: blocker Used on issues or pulls that block work from being released. label Jun 2, 2020
@nerrad
Copy link
Contributor

nerrad commented Jun 2, 2020

I just realized that I hadn't prioritized this. I think we need to consider this a blocker to releasing 2.7.0 because that's the tag that will be merged to WC Core and thus introduce the significant weight increases in the bundle-sizes for various existing built files (All products, and filter blocks).

I'd like to avoid duplicating work in creating our own notice components in lieue of the WordPress package. @senadir , I think you were working on a way to address this?

@Aljullu
Copy link
Contributor

Aljullu commented Jun 9, 2020

Even after the improvements from #2664, the size of all-products-frontend.js still increased from 14,85KB (WC Core 5.2) to 38,41KB (WC Blocks 2.7). Should we reopen this issue to investigate if we can improve this further?

@senadir
Copy link
Member Author

senadir commented Jun 9, 2020

Removing notice will potentially remove other dependencies, but if we ever found ourself doing animations we can reuse some of those things, you can create an issue, but I'm certain the extra weight is from the effect-ful imports in @wordpress/components that we can’t get rid of unless we remove @wordpress/components (or try something similar to #2664).

@nerrad
Copy link
Contributor

nerrad commented Jun 9, 2020

Keep in mind we use the snackbar notices which in turn uses react-spring. I think at some point we're going to have to look investigate using lazy loading more. That way some of the extra package dependencies could be pulled in only when needed.

So while I'm 💯 on continuing to look at where we can reduce bundlesizes, I'm not convinced removing our usage of WordPress notices is the way to go because it means we just have to reproduce all of that code for what I imagine will likely be marginal gain.

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
focus: performance The issue/PR is related to performance. status: blocker Used on issues or pulls that block work from being released.
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants