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

Debug mode support for assets #484

Closed
wants to merge 31 commits into
base: master
from

Conversation

Projects
None yet
2 participants
@actual-saurabh
Contributor

actual-saurabh commented Mar 29, 2018

Summary of Changes:

  1. Extend support for SCRIPT_DEBUG to all enqueued assets, whenever available, including for
    1. bundled third-party assets
    2. assets loaded by CDN
  2. Optimise enqueuing mechanism by replacing multiple function calls and variables with single global constants.

@actual-saurabh actual-saurabh deleted the actual-saurabh:feature-debug-enqueue branch Apr 17, 2018

@thomasplevy

This comment has been minimized.

Member

thomasplevy commented Apr 17, 2018

@actual-saurabh Why'd you close this?

@actual-saurabh actual-saurabh restored the actual-saurabh:feature-debug-enqueue branch Apr 17, 2018

@actual-saurabh

This comment has been minimized.

Contributor

actual-saurabh commented Apr 17, 2018

Oh, sorry. I was cleaning up branches on my fork and deleted this one too. Restored the branch. Should I pull from master, merge and update the pull request?

@thomasplevy

This comment has been minimized.

Member

thomasplevy commented Apr 17, 2018

please! I hadn't merged it in yet but it looks solid and I'd like to get it into core

@thomasplevy

This comment has been minimized.

Member

thomasplevy commented Apr 24, 2018

@actual-saurabh there's an issue with this... most of the JS files are only added to the assets directory as minified versions. The gulp tasks that concat & minify JS need to create a concatenated only non minified version in order for this to fully function, otherwise with script debug enabled most of the scripts will 404

@actual-saurabh

This comment has been minimized.

Contributor

actual-saurabh commented Apr 25, 2018

I have added a new task to minify vendor scripts to maintain consistency. See: https://github.com/actual-saurabh/lifterlms/tree/feature-debug-enqueue/assets/js/vendor

I then added this as a dependency in the original task. I also modified the original process-scripts task to also copy over unminified assets: https://github.com/actual-saurabh/lifterlms/tree/feature-debug-enqueue/assets/js

@actual-saurabh

This comment has been minimized.

Contributor

actual-saurabh commented Apr 25, 2018

@thomasplevy #504, #505 describe what look like legacy remnants in assets that don't seem to be used anymore. Can I go ahead and delete them?

Apart from these two, #503 describes two other scripts that have minified version in the distribution but no unminified sources to build from.

@thomasplevy thomasplevy self-assigned this May 1, 2018

@thomasplevy thomasplevy added this to the Scrub May 2018 milestone May 1, 2018

@thomasplevy thomasplevy closed this May 1, 2018

@actual-saurabh actual-saurabh deleted the actual-saurabh:feature-debug-enqueue branch Jun 15, 2018

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment