Skip to content
This repository has been archived by the owner on Dec 5, 2020. It is now read-only.

Enabling autoprefixer on a theme breaks the deploy #274

Closed
gcmznt opened this issue Apr 2, 2019 · 6 comments · Fixed by #287
Closed

Enabling autoprefixer on a theme breaks the deploy #274

gcmznt opened this issue Apr 2, 2019 · 6 comments · Fixed by #287

Comments

@gcmznt
Copy link

gcmznt commented Apr 2, 2019

Enabling autoprefixer on Minium theme breaks the deploy just after the postCSS gulp plugin

@wincent
Copy link
Contributor

wincent commented Apr 5, 2019

@gcmznt: what does the breakage look like? (ie. anything printed to the console?)

@gcmznt
Copy link
Author

gcmznt commented Apr 5, 2019

I had this problem running the deploy task via gradle, running the task via gulp it works.
With the gradle deploy task the gulp build just fail silently when autoprefixer is piped

@wincent
Copy link
Contributor

wincent commented Apr 8, 2019

@gcmznt: I'm having a bit of trouble reproducing this, because I am not sure what version you're working with at the moment.

At least on "master" (which uses v9 of the themes toolkit), I tried turning on autoprefixer in modules/apps/frontend-theme-fjord/frontend-theme-fjord (I am not sure where to get this Minium theme that you mentioned).

diff --git a/modules/apps/frontend-theme-fjord/frontend-theme-fjord/package.json b/modules/apps/frontend-theme-fjord/frontend-theme-fjord/package.json
index e9b18ba4b6f6..538ec5bde76a 100644
--- a/modules/apps/frontend-theme-fjord/frontend-theme-fjord/package.json
+++ b/modules/apps/frontend-theme-fjord/frontend-theme-fjord/package.json
@@ -11,7 +11,10 @@
                "baseTheme": "styled",
                "distName": "fjord-theme",
                "rubySass": false,
-               "version": "7.1"
+               "version": "7.1",
+               "postcss": [
+                       "autoprefixer"
+               ]
        },
        "main": "package.json",
        "name": "liferay-fjord-theme",

When I run gradlew clean deploy I see the deploy finish without errors and it includes this output suggesting that autoprefixer was invoked:

[12:11:11] gulp-postcss: main.css
autoprefixer: /Users/greghurrell/code/portal/liferay-portal/modules/apps/frontend-theme-fjord/frontend-theme-fjord/build/_css/main.css:2568:9: You should write display: flex by final spec instead of display: box
autoprefixer: /Users/greghurrell/code/portal/liferay-portal/modules/apps/frontend-theme-fjord/frontend-theme-fjord/build/_css/main.css:2635:3: You should write display: flex by final spec instead of display: box
autoprefixer: /Users/greghurrell/code/portal/liferay-portal/modules/apps/frontend-theme-fjord/frontend-theme-fjord/build/_css/main.css:2662:5: You should write display: flex by final spec instead of display: box
Done in 4.79s.

And the server log shows the deploy starting and finishing:

2019-04-08 09:37:14.770 INFO  [fileinstall-/Users/greghurrell/code/portal/bundles/osgi/war][BaseAutoDeployListener:50] Themes for /Users/greghurrell/code/portal/bundles/tomcat-9.0.17/temp/20190408093712799GERBSMMI/fjord-theme.war copied successfully
2019-04-08 09:37:18.079 INFO  [Refresh Thread: Equinox Container: a100ea15-3ee7-459e-9f01-11988f451df1][ThemeHotDeployListener:108] 1 theme for fjord-theme is available for use
2019-04-08 09:37:18.118 INFO  [Refresh Thread: Equinox Container: a100ea15-3ee7-459e-9f01-11988f451df1][BundleStartStopLogger:39] STARTED fjord-theme_3.0.0 [930]

Can you let me know which version of the portal you're working in, and which version of the themes toolkit? And if you think there is anything specific to the Minium theme that is necessary to repro this, let me know how to get a hold of it? Thanks.

@wincent
Copy link
Contributor

wincent commented Apr 8, 2019

Actually, scratch that! I misread the deploy log: it actually says

> Task :apps:frontend-theme-fjord:frontend-theme-fjord:deploy NO-SOURCE

It's supposed to say something like:

Files of project ':apps:frontend-theme-fjord:frontend-theme-fjord' deployed to /Users/greghurrell/code/portal/bundles/deploy

instead of "NO SOURCE".

I can take it from here!

@wincent
Copy link
Contributor

wincent commented Apr 8, 2019

Temporary workaround until I figure out how/why gulp-plumber is breaking the build; comment out this line:

in liferay-portal/modules/node_modules/liferay-theme-tasks/tasks/build/compile-css.js.

wincent added a commit that referenced this issue Apr 8, 2019
When deploying a theme using this config in the package.json:

    "liferayTheme": {
            "baseTheme": "styled",
            "distName": "fjord-theme",
            "rubySass": false,
            "version": "7.1",
            "postcss": [
                    "autoprefixer"
            ]
    },

the deploy would silently fail because "gulp-plumber" would swallow the
error. If I am reading this background material on plumber correctly:

- https://github.com/floatdrop/gulp-plumber
- gulpjs/gulp#91
- https://gist.github.com/floatdrop/8269868

the intent of the plug-in is to monkey patch the Gulp pipe objects such
that an error in one file won't prevent the others from being processed.
We added it in this repo in commit 9200922 (Jan
2016, "Use gulp-plumber for r2 task so that sass-parse errors don't
2abort build process").

In practice, its black magic is causing deploys to fail inscrutably, so
we're dropping it. In the event of an error, let's fail fast instead.
And note, in this concrete instance, I don't think there was even an
error being thrown in the autoprefixer build at all (based on my
`console.log()`-ing around to see at what point it was going off the
rails; the postcss run is finishing just fine.

Note that we could also switch our old v0.6.6 version of this dependency
to the latest, v1.2.1, but there's nothing in the diff that stands out
as being likely to fix this issue, and I'll be much happier if we can
jettison this complexity:

    floatdrop/gulp-plumber@v0.6.6...v1.2.1

Test plan: In portal, in modules/apps/frontend-theme-fjord/frontend-theme-fjord,
turn on autoprefixer with the config mentioned above. Run both `gradlew
clean deploy` and also (the equivalent):

```
yarn run gulp deploy \
  --css-common-path ./build_gradle/frontend-css-common \
  --styled-path ../../frontend-theme/frontend-theme-styled/src/main/resources/META-INF/resources/_styled \
  --unstyled-path ../../frontend-theme/frontend-theme-unstyled/src/main/resources/META-INF/resources/_unstyled
```

and see the build output continue past the "autoprefixer" lines to
finish with:

```
[13:21:39] Finished 'deploy' after 5.27 s
✨  Done in 5.97s.
```

Likewise, see the deployment reflected in the server log:

```
1 theme for fjord-theme is available for use
```

Closes: #274
wincent added a commit that referenced this issue Apr 8, 2019
When deploying a theme using this config in the package.json:

    "liferayTheme": {
            "baseTheme": "styled",
            "distName": "fjord-theme",
            "rubySass": false,
            "version": "7.1",
            "postcss": [
                    "autoprefixer"
            ]
    },

the deploy would silently fail because "gulp-plumber" would swallow the
error. If I am reading this background material on plumber correctly:

- https://github.com/floatdrop/gulp-plumber
- gulpjs/gulp#91
- https://gist.github.com/floatdrop/8269868

the intent of the plug-in is to monkey patch the Gulp pipe objects such
that an error in one file won't prevent the others from being processed.
We added it in this repo in commit 9200922 (Jan
2016, "Use gulp-plumber for r2 task so that sass-parse errors don't
abort build process").

In practice, its black magic is causing deploys to fail inscrutably, so
we're dropping it. In the event of an error, let's fail fast instead.
And note, in this concrete instance, I don't think there was even an
error being thrown in the autoprefixer build at all (based on my
`console.log()`-ing around to see at what point it was going off the
rails; the postcss run is finishing just fine.

Note that we could also switch our old v0.6.6 version of this dependency
to the latest, v1.2.1, but there's nothing in the diff that stands out
as being likely to fix this issue, and I'll be much happier if we can
jettison this complexity:

    floatdrop/gulp-plumber@v0.6.6...v1.2.1

Test plan: In portal, in modules/apps/frontend-theme-fjord/frontend-theme-fjord,
turn on autoprefixer with the config mentioned above. Run both `gradlew
clean deploy` and also (the equivalent):

```
yarn run gulp deploy \
  --css-common-path ./build_gradle/frontend-css-common \
  --styled-path ../../frontend-theme/frontend-theme-styled/src/main/resources/META-INF/resources/_styled \
  --unstyled-path ../../frontend-theme/frontend-theme-unstyled/src/main/resources/META-INF/resources/_unstyled
```

and see the build output continue past the "autoprefixer" lines to
finish with:

```
[13:21:39] Finished 'deploy' after 5.27 s
✨  Done in 5.97s.
```

Likewise, see the deployment reflected in the server log:

```
1 theme for fjord-theme is available for use
```

Closes: #274
wincent added a commit that referenced this issue Apr 8, 2019
Fixes:

    packages/liferay-theme-tasks/tasks/build/compile-css.js
      11:7  error  'plugins' is assigned a value but never used  no-unused-vars
wincent added a commit that referenced this issue Apr 8, 2019
This is the 8.x equivalent of:

    #287

When deploying a theme using this config in the package.json:

    "liferayTheme": {
            "baseTheme": "styled",
            "distName": "fjord-theme",
            "rubySass": false,
            "version": "7.1",
            "postcss": [
                    "autoprefixer"
            ]
    },

the deploy would silently fail because "gulp-plumber" would swallow the
error. If I am reading this background material on plumber correctly:

- https://github.com/floatdrop/gulp-plumber
- gulpjs/gulp#91
- https://gist.github.com/floatdrop/8269868

the intent of the plug-in is to monkey patch the Gulp pipe objects such
that an error in one file won't prevent the others from being processed.
We added it in this repo in commit 9200922 (Jan
2016, "Use gulp-plumber for r2 task so that sass-parse errors don't
abort build process").

In practice, its black magic is causing deploys to fail inscrutably, so
we're dropping it. In the event of an error, let's fail fast instead.
And note, in this concrete instance, I don't think there was even an
error being thrown in the autoprefixer build at all (based on my
`console.log()`-ing around to see at what point it was going off the
rails; the postcss run is finishing just fine.

Note that we could also switch our old v0.6.6 version of this dependency
to the latest, v1.2.1, but there's nothing in the diff that stands out
as being likely to fix this issue, and I'll be much happier if we can
jettison this complexity:

    floatdrop/gulp-plumber@v0.6.6...v1.2.1

Test plan: In portal, in modules/apps/frontend-theme-fjord/frontend-theme-fjord,
turn on autoprefixer with the config mentioned above. Run both `gradlew
clean deploy` and also (the equivalent):

```
yarn run gulp deploy \
  --css-common-path ./build_gradle/frontend-css-common \
  --styled-path ../../frontend-theme/frontend-theme-styled/src/main/resources/META-INF/resources/_styled \
  --unstyled-path ../../frontend-theme/frontend-theme-unstyled/src/main/resources/META-INF/resources/_unstyled
```

and see the build output continue past the "autoprefixer" lines to
finish with:

```
[13:21:39] Finished 'deploy' after 5.27 s
✨  Done in 5.97s.
```

Likewise, see the deployment reflected in the server log:

```
1 theme for fjord-theme is available for use
```

Related: #274
julien added a commit that referenced this issue Apr 8, 2019
fix: silent failures when using autoprefixer (9.x) (#274)
julien added a commit that referenced this issue Apr 8, 2019
fix: silent failures when using autoprefixer (8.x) (#274)
@wincent
Copy link
Contributor

wincent commented Apr 9, 2019

Just published v8.0.6 and v9.0.0-beta.1 which contain the fix for this.

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants