fix: simplify npm run watch-sass so that it is more reliable

The implementation of `npm run watch-sass` was trying really hard
to recompile precisely only the Sass that needed to be recompiled, but in
order to do so, it had to spin up several `watchmedo` processes
per theme. These processes would trigger one another sometimes, leading
to infinite recompilation loops.

Rather than figure out all the dependency directions and messing with
`watchmedo`, I've opted to simplify the script to invoke a single
`watchmedo` process per theme. A single theme recompiles within
seconds, so I think this is a good compromise, one which makes the
script easier to reason about will help me move pass this legacy
assets work.
This commit is contained in:
Kyle D. McCormick
2024-03-26 17:44:17 -04:00
committed by Kyle McCormick
parent 5a785482ab
commit 3b8d018e4b

View File

@@ -2,7 +2,6 @@
# Wait for changes to Sass, and recompile.
# Invoke from repo root as `npm run watch-sass`.
# This script tries to recompile the minimal set of Sass for any given change.
# By default, only watches default Sass.
# To watch themes too, provide colon-separated paths in the EDX_PLATFORM_THEME_DIRS environment variable.
@@ -46,34 +45,24 @@ echo_quoted_cmd() {
start_sass_watch() {
# Start a watch for .scss files in a particular dir. Run in the background.
# start_sass_watch NAME_FOR_LOGGING WATCH_ROOT_PATH HANDLER_COMMAND
# Usage: start_sass_watch NAME_FOR_LOGGING HANDLER_COMMAND WATCH_ROOT_PATHS...
local name="$1"
local path="$2"
local handler="$3"
log "Starting watcher for $name:"
local handler="$2"
shift 2
local paths=("$@")
section "Starting Sass watchers for $name:"
# Note: --drop means that we should ignore any change events that happen during recompilation.
# This is good because it means that if you edit 3 files, we won't fire off three simultaneous compiles.
# It's not perfect, though, because if you change 3 files, only the first one will trigger a recompile,
# so depending on the timing, your latest changes may or may not be picked up. We accept this as a reasonable
# tradeoff. Some watcher libraries are smarter than watchdog, in that they wait until the filesystem "settles"
# before firing off a the recompilation. This sounds nice but did not seem worth the effort for legacy assets.
local watch_cmd=(watchmedo shell-command -v --patterns '*.scss' --recursive --drop --command "$handler" "$path")
local watch_cmd=(watchmedo shell-command -v --patterns '*.scss' --recursive --drop --command "$handler" "${paths[@]}")
echo_quoted_cmd "${watch_cmd[@]}"
"${watch_cmd[@]}" &
}
clean_up() {
# Kill all background processes we started.
# Since they're all 'watchmedo' instances, we can just use killall.
log "Stopping all watchers:"
local stop_cmd=(killall watchmedo)
echo_quoted_cmd "${stop_cmd[@]}"
"${stop_cmd[@]}" || true
log "Watchers stopped."
}
warning "'npm run watch-sass' in edx-platform is experimental. Use at your own risk."
# Verify execution environment.
if [[ ! -d common/static/sass ]] ; then
error 'This command must be run from the root of edx-platform!'
exit 1
@@ -85,81 +74,51 @@ if ! type watchmedo 1>/dev/null 2>&1 ; then
exit 1
fi
trap clean_up EXIT
# Reliably kill all child processes when script is interrupted (Ctrl+C) or otherwise terminated.
trap "exit" INT TERM
trap "kill 0" EXIT
# Start by compiling all watched Sass right away, mirroring the behavior of webpack --watch.
section "COMPILING SASS:"
npm run compile-sass
echo
echo
section "STARTING DEFAULT SASS WATCHERS:"
# Changes to LMS Sass require a full recompilation, since LMS Sass can be used in CMS and in themes.
start_sass_watch "default LMS Sass" \
# Watch default Sass.
# If it changes, then recompile the default theme *and* all custom themes,
# as custom themes' Sass is based on the default Sass.
start_sass_watch "default theme" \
'npm run compile-sass-dev' \
lms/static/sass \
'npm run compile-sass-dev'
# Changes to default cert Sass only require recompilation of default cert Sass, since cert Sass
# cannot be included into LMS, CMS, or themes.
start_sass_watch "default certificate Sass" \
lms/static/certificates/sass \
'npm run compile-sass-dev -- --skip-cms --skip-themes'
# Changes to CMS Sass require recompilation of default & themed CMS Sass, but not LMS Sass.
start_sass_watch "default CMS Sass" \
cms/static/sass \
'npm run compile-sass-dev -- --skip-lms'
# Sass changes in common, node_modules, and xmodule all require full recompilations.
start_sass_watch "default common Sass" \
common/static \
'npm run compile-sass-dev'
start_sass_watch "node_modules Sass" \
node_modules \
'npm run compile-sass-dev'
start_sass_watch "builtin XBlock Sass" \
xmodule/assets \
'npm run compile-sass-dev'
xmodule/assets
export IFS=";"
# Watch each theme's Sass.
# If it changes, only recompile that theme.
export IFS=":"
for theme_dir in ${EDX_PLATFORM_THEME_DIRS:-} ; do
for theme_path in "$theme_dir"/* ; do
theme_name="${theme_path#"$theme_dir/"}"
lms_sass="$theme_path/lms/static/sass"
cert_sass="$theme_path/lms/static/certificates/sass"
cms_sass="$theme_path/cms/static/sass"
if [[ -d "$lms_sass" ]] || [[ -d "$cert_sass" ]] || [[ -d "$cms_sass" ]] ; then
section "STARTING WATCHERS FOR THEME '$theme_name':"
fi
# Changes to a theme's LMS Sass require that the full theme is recompiled, since LMS
# Sass is used in certs and CMS.
theme_watch_dirs=()
if [[ -d "$lms_sass" ]] ; then
start_sass_watch "$theme_name LMS Sass" \
"$lms_sass" \
"npm run compile-sass-dev -- -T $theme_dir -t $theme_name --skip-default"
theme_watch_dirs+=("$lms_sass")
fi
# Changes to a theme's certs Sass only requires that its certs Sass be recompiled.
if [[ -d "$cert_sass" ]] ; then
start_sass_watch "$theme_name certificate Sass" \
"$cert_sass" \
"npm run compile-sass-dev -- -T $theme_dir -t $theme_name --skip-default --skip-cms"
theme_watch_dirs+=("$cert_sass")
fi
# Changes to a theme's CMS Sass only requires that its CMS Sass be recompiled.
if [[ -d "$cms_sass" ]] ; then
start_sass_watch "$theme_name CMS Sass" \
"$cms_sass" \
"npm run compile-sass-dev -- -T $theme_dir -t $theme_name --skip-default --skip-lms"
theme_watch_dirs+=("$cms_sass")
fi
# A directory is a theme if it as LMS Sass *and/or* CMS Sass *and/or* certificate Sass.
if [[ -n "${theme_watch_dirs[*]}" ]] ; then
start_sass_watch "theme '$theme_name'" \
"npm run compile-sass-dev -- -T $theme_dir -t $theme_name --skip-default" \
"${theme_watch_dirs[@]}"
fi
done
done
# Wait until interrupted/terminated.
sleep infinity &
echo
echo "Watching Open edX LMS & CMS Sass for changes."