Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fixed gulp build:production does not generate unminified/unoptimized CSS/JS files. #11

Closed
wants to merge 4 commits into from

Conversation

sun
Copy link
Member

@sun sun commented Aug 23, 2018

Repeats #3 after it was reverted in 5895511

Problem

  • When running gulp build to set up a new instance of an existing project (e.g. during a deployment process), then only minified/optimized files are generated, causing 404 errors and a broken front-end in case the application searches for unminified files.

Goal

  • Simplify project setup and deployment processes.

Proposed solution

  1. Always write out the generated unminified/unoptimized files before the content is (potentially) minified/optimized.
    • As visible in the diff, the precompiled but yet unminified/unoptimized files are available right there and can be written out to disk with zero additional cost.

Notes

  • Having the unminified files in production is not a problem; they're simply not used.

  • For production: false this PR causes a double write to the same file, which may be optimized in the future, but does not present a problem right now.

Details

  • Generating the unmodified/unoptimized files takes twice the time for the case of build:production even though the necessary output was generated already.
  • It doesn't appear to be possible to run build and build:production in sequence, because both invoke clean as first operation.
  • Running tasks after each other compiles the files separately from scratch for each task. That is a waste of time and resources.

Copy link
Contributor

@LucaPipolo LucaPipolo left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@sun — make sense for me.
@colourgarden, do you want to double check before merge? 🔍

@colourgarden
Copy link
Contributor

Is this change still relevant? What project do we need it for?

Previous Slack discussion here.

@sun
Copy link
Member Author

sun commented Aug 2, 2019

Yes, it's still relevant when using gulp-task-collection in an application that allows to switch asset file loading between minified/optimized and original source files through a configuration setting, such as the SCRIPT_DEBUG constant in WordPress and similar settings in other systems.

@sun sun closed this May 8, 2020
@sun sun reopened this May 8, 2020
@sun
Copy link
Member Author

sun commented May 8, 2020

Unoptimized source files obviously cannot be loaded from assets, as they are not yet compiled in there.

@sun
Copy link
Member Author

sun commented May 13, 2020

This is still needed to properly handle e.g. the SCRIPT_DEBUG constant in WordPress. The constant can be flipped at any time and the site should still load correctly when plugins and themes are requesting the minified or non-minified assets.

We've started to conditionally load separate files in some projects depending on the constant already, but some of the projects are now trying to load the files from the uncompiled assets folder, which obviously cannot work if they contain e.g. SCSS instead of CSS files.

A separate, non-standard build task does not make sense from my perspective, as these files should be generated by default during the regular compilation. Writing out the intermediate files before they get optimized and minified is the most sensible solution, which also does not negatively affect the build time.

@wrux
Copy link
Contributor

wrux commented Aug 5, 2020

Closing this in favour of #33

@wrux wrux closed this Aug 5, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants