Skip to content

Commit

Permalink
fix: simplify npm run watch-sass so that it is more reliable
Browse files Browse the repository at this point in the history 
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.
  • Loading branch information
@kdmccormick
kdmccormick committed Apr 4, 2024
1 parent fe13884 commit 966f2dc
Showing 1 changed file with 30 additions and 71 deletions.
101 changes: 30 additions & 71 deletions scripts/watch_sass.sh
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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.
Expand Down Expand Up @@ -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
Expand All @@ -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."
Expand Down

0 comments on commit 966f2dc

Please sign in to comment.