Openfoodnetwork: Duplicate CSS definitions from use of css_splitter

[pmackay]

Original from Paul:

I've been looking at the CSS structure of OFN a bit. css_splitter is used but it seems like many CSS blocks are being imported twice because:

• all.css uses @import '*';
• all_split2.css uses *= require 'darkswarm/all'

Wont both of these import all the darkswarm CSS files?

Description

Our all.css file includes all_split2.css which is entirely contained in all.css already. So the include is unnecessary and lets visitors wait longer before they can see the page.

Expected Behavior

No duplicate CSS code is loaded.

Actual Behavior

180 kb of duplicate CSS code is loaded via all_split2.css.

Steps to Reproduce

1. Load any page and observe the css include links: curl -s https://openfoodnetwork.org.au/ | grep 'all.*css'
2. Compare the two files, for example: sed -i 's/;/\n/g' *.css; meld *.css

Severity

s3 as it affects all users. It doesn't prevent most people from using the platform, but can put customers off that have a slow internet connection (bad mobile signal).

Your Environment

Observed on the Australia production server:

• all-c448b4e380da3e7c7b14b01981eab7bd.css: 377K
• all_split2.css: 180K

all.css seems to contain third party styles like Foundation, all_split2.css does not.

Possible Fix

Remove the css splitter and references to all_split2.css.

[oeoeaio]

That's entirely possible. Probably a clearer file structure, and standards around file inclusion (as you've proposed) would help us resolve these problems.

[mkllnk]

I don't know if rails is smart enough to filter duplicates.

[myriamboure]

@mkllnk is this still a problem? If yes do you want to specify the issue and what we could do with that? If a new dev can take it maybe if that's important...

[mkllnk]

Yes, I can confirm that this is still a problem. I'll apply the issue template.

[luisramos0]

In #2540, we are removing precompile entry for split2 here.

The all_split2.css is still deployed. I thought we should mention it on this issue for reference.

[Vadlusk]

Does that mean I can safely remove all_split2? That sounds like something I can handle if it's as simple as deleting this.

[luisramos0]

Hey Adam!

Lets fix this! It's 218kb of unnecessary stuff. Is it unnecessary?
css_splitter splits css files with more than 4096 rules so that all rules are used in internet explorer 9 or below.
The question is: are we below this number and is this split2.css file not doing anything?
I think we need to REcheck this.

If we are sure of the above, we can remove css_splitter.
I tried it here #2633 but for some reason the split2css file is still there...

So the task would be:

• verify all_split2.css is contained in all.css
• make #2633 work

Let us know if you pick this up ;-)

[Vadlusk]

I am beginning to look into it now. Thanks for the opportunity!

[Vadlusk]

I'm having trouble comparing the files.
When I run the commands in the initial comment under 'Steps to Reproduce,'
the second command sed -i 's/;/\n/g' *.css; meld *.css
produces
sed: 1: "*.css": invalid command code *.
Meld opens fine despite this error, but has nothing to compare.
Is this the right way to compare these files?

[luisramos0]

yeah, it depends on your system, mac, linux, bash, ksh...

I am on mac and Ive done this:
tr ';' '\n' < all.css > new_all.css
tr ';' '\n' < split.css > new_split.css

and then looked at the files with IntelliJ diff.

I can confirm the split.css file is totally included in the all.css file.
SO, we can remove it :-)

[luisramos0]

@Vadlusk 🎉