Duplicate resulting CSS

[yuchi]

Note: This issue has not been verified directly with Clay build process, only through building a theme for Liferay DXP 7.1

Looks like you are using two different ways to import Bootstrap SCSS in your main entry points (base, atlas, bootstrap).

In bootstrap.scss you load it as:

@import "bootstrap/bootstrap";

In atlas.scss and base.scss you load it as:

@import "bootstrap"; // resolves to './bootstrap.scss' !!!

This means that atlas.scss and base.scss are not loading Bootstrap directly, but pass through bootstrap.scss which in turn actually loads Bootstrap for them.

This approach has no issues here on Clay CSS repo, where bootstrap.scss has no content outside the import.

In contrast, in Liferay DXP themes those files have their // INSERT … comments replaced with imports to the theme custom variables and extensions. This means that if the theme is using atlast or base they will get the following structure (indentation artificially added to make imports clearer):

// build/css/clay.scss
// @import "clay/base";

    // build/css/clay/base.scss
    @import url(font_awesome.css);
    @import "../clay_variables";

    @import "functions/_global-functions";

    @import "variables/_bs4-variable-overwrites";

    //@import "bootstrap";

        // build/css/clay/bootstrap.scss
        @import url(font_awesome.css);
        @import "../clay_variables";

        @import "bootstrap/bootstrap";

        @import "variables";

        @import "../liferay_variables_custom";
        @import "../liferay_variables";
        @import "bourbon";
        @import "../clay_custom";
        @import "../liferay_custom";

    @import "_variables";

    @import "_mixins";

    @import "_components";

    @import "variables";

    @import "../liferay_variables_custom";
    @import "../liferay_variables";
    @import "bourbon";
    @import "../clay_custom";
    @import "../liferay_custom";

You can easily see how this produces duplicate CSS in the final output.

Proposal (PR coming soon)

All three entrypoints should load Bootstrap in the way bootstrap.scss is doing.

[yuchi]

/cc @gretaaaa which found the issue and helped me debug the problem.

[yuchi]

Sorry if we didn’t provide a PR. Thanks for solving it.

[yuchi]

Now that I see that PR, why did you squash all imports into the entrypoints!?

@marcoscv-work @carloslancha

[marcoscv-work]

Hey @yuchi When you wrote "All three entrypoints should load Bootstrap in the way bootstrap.scss is doing." did not you mean to use the final bootstrap/bootstrap.scss imports?

I think @pat270 and I understood it in this way

[yuchi]

Oh! My wording was terrible now that I think of it! Sorry for the confusion. In my personal opinion all three entry points should use this:

@import "bootstrap/bootstrap";

That way all of them have the same exact behavior, without requiring any duplication. In a SCSS they would be symlinks.

[pat270]

@yuchi, you make a good point. The reason I didn't do it that way was because I'm unable to modify the Bootstrap source files and we will need to for https://github.com/liferay/clay/issues/1270. There's some magic that happens when we build the Clay project that resets any changes made.

I suppose I could have made a separate file and have all the entry points use that file instead. We could also change the build task to use the Bootstrap source from clay-css instead of npm.

[yuchi]

[…] because I'm unable to modify the Bootstrap source files […]

Wait. I’m talking about this bootstrap.scss. There’s no need to change Bootstrap sources, I’m talking about your main entry points (namely clay-css/src/scss/base.scss, clay-css/src/scss/atlas.scss and clay-css/src/scss/bootstrap.scss)

[pat270]

@yuchi, Right I was also referring to those files. I went off on a tangent regarding compile errors with variable resets using Ruby/Dart Sass. The solution for #1270 I had in mind was way too complicated and decided to scrap the idea. We can change the imports on clay-css/src/scss/base.scss, clay-css/src/scss/atlas.scss, and clay-css/src/scss/bootstrap.scss to:

@import "bootstrap/bootstrap";

[yuchi]

IMHO it’s clearer and less obtrusive approach. Just my 2¢ anyway.