This repository has been archived by the owner on Dec 5, 2020. It is now read-only.
-
Notifications
You must be signed in to change notification settings - Fork 28
fix: silent failures when using autoprefixer (9.x) (#274) #287
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
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
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 pull request
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
Just started reviewing :) |
Sign up for free
to subscribe to this conversation on GitHub.
Already have an account?
Sign in.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
When deploying a theme using this config in the package.json:
the deploy would silently fail because "gulp-plumber" would swallow the error. If I am reading this background material on plumber correctly:
pipe
method from gulp.src gulpjs/gulp#91the 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):and see the build output continue past the "autoprefixer" lines to finish with:
Likewise, see the deployment reflected in the server log:
Closes: #274