Change how theme style packs are loaded

Load the `common` style pack, and then charge the style pack for the current
skin, independent from any selected JS pack.

app/controllers/application_controller.rb

@@ -19,6 +19,7 @@ class ApplicationController < ActionController::Base
   helper_method :current_session
   helper_method :current_flavour
   helper_method :current_skin
+  helper_method :current_theme
   helper_method :single_user_mode?
   helper_method :use_seamless_external_login?
   helper_method :omniauth_only?

app/controllers/concerns/theming_concern.rb

@@ -4,7 +4,7 @@ module ThemingConcern
   extend ActiveSupport::Concern
 
   def use_pack(pack_name)
-    @theme = resolve_pack_with_common(Themes.instance.flavour(current_flavour), pack_name, current_skin)
+    @theme = resolve_pack(Themes.instance.flavour(current_flavour), pack_name)
   end
 
   private
@@ -18,29 +18,22 @@ module ThemingConcern
     [current_user&.setting_skin, Setting.skin, 'default'].find { |skin| skins.include?(skin) }
   end
 
-  def valid_pack_data?(data, pack_name)
+  def current_theme
-    data['pack'].is_a?(Hash) && data['pack'][pack_name].present?
+    # NOTE: this is slightly different from upstream, as it's a derived value used
+    # for the sole purpose of pointing to the appropriate stylesheet pack
+    "skins/#{current_flavour}/#{current_skin}"
   end
 
-  def nil_pack(data)
+  def resolve_pack(data, pack_name)
-    {
+    pack_data = {
       flavour: data['name'],
       pack: nil,
       preload: nil,
-      skin: nil,
-      supported_locales: data['locales'],
-    }
-  end
-
-  def pack(data, pack_name, skin)
-    pack_data = {
-      flavour: data['name'],
-      pack: pack_name,
-      preload: nil,
-      skin: nil,
       supported_locales: data['locales'],
     }
+    return pack_data unless data['pack'].is_a?(Hash) && data['pack'][pack_name].present?
 
+    pack_data[:pack] = pack_name
     return pack_data unless data['pack'][pack_name].is_a?(Hash)
 
     pack_data[:pack] = nil unless data['pack'][pack_name]['filename']
@@ -49,22 +42,6 @@ module ThemingConcern
     pack_data[:preload] = [preloads] if preloads.is_a?(String)
     pack_data[:preload] = preloads if preloads.is_a?(Array)
 
-    if skin != 'default' && data['skin'][skin]
-      pack_data[:skin] = skin if data['skin'][skin].include?(pack_name)
-    elsif data['pack'][pack_name]['stylesheet']
-      pack_data[:skin] = 'default'
-    end
-
     pack_data
   end
-
-  def resolve_pack(data, pack_name, skin)
-    pack(data, pack_name, skin) if valid_pack_data?(data, pack_name)
-  end
-
-  def resolve_pack_with_common(data, pack_name, skin = 'default')
-    result = resolve_pack(data, pack_name, skin) || nil_pack(data)
-    result[:common] = resolve_pack(data, 'common', skin)
-    result
-  end
 end

app/javascript/flavours/glitch/common.js

@@ -0,0 +1,12 @@
+import Rails from '@rails/ujs';
+import 'font-awesome/css/font-awesome.css';
+
+export function start() {
+  require.context('@/images/', true);
+
+  try {
+    Rails.start();
+  } catch (e) {
+    // If called twice
+  }
+}

app/javascript/flavours/glitch/packs/application.js

@@ -1,8 +1,12 @@
 import 'packs/public-path';
+
+import { start } from 'flavours/glitch/common';
 import { loadLocale } from 'flavours/glitch/locales';
 import main from "flavours/glitch/main";
 import { loadPolyfills } from 'flavours/glitch/polyfills';
 
+start();
+
 loadPolyfills()
   .then(loadLocale)
   .then(main)

app/javascript/flavours/glitch/packs/common.js

@@ -1,9 +1,8 @@
+/* This file is a hack to have something more reliable than the upstream `common` tag
+  that is implicitly generated as the common chunk through webpack's `splitChunks` config */
+
 import 'packs/public-path';
 import 'font-awesome/css/font-awesome.css';
-import Rails from '@rails/ujs';
-import 'flavours/glitch/styles/index.scss';
 
-Rails.start();
+// This is a hack to ensures that webpack compiles our images.
-
-//  This ensures that webpack compiles our images.
 require.context('../images', true);

app/javascript/flavours/glitch/packs/public.tsx

@@ -10,6 +10,7 @@ import Rails from '@rails/ujs';
 import axios from 'axios';
 import { throttle } from 'lodash';
 
+import { start } from 'flavours/glitch/common';
 import { timeAgoString } from 'flavours/glitch/components/relative_timestamp';
 import emojify from 'flavours/glitch/features/emoji/emoji';
 import loadKeyboardExtensions from 'flavours/glitch/load_keyboard_extensions';
@@ -19,6 +20,8 @@ import ready from 'flavours/glitch/ready';
 
 import 'cocoon-js-vanilla';
 
+start();
+
 const messages = defineMessages({
   usernameTaken: {
     id: 'username.taken',

app/javascript/flavours/glitch/packs/share.jsx

@@ -1,10 +1,13 @@
 import 'packs/public-path';
 import { createRoot } from 'react-dom/client';
 
+import { start } from 'flavours/glitch/common';
 import ComposeContainer from 'flavours/glitch/containers/compose_container';
 import { loadPolyfills } from 'flavours/glitch/polyfills';
 import ready from 'flavours/glitch/ready';
 
+start();
+
 function loaded() {
   const mountNode = document.getElementById('mastodon-compose');
 

app/javascript/packs/common.js

@@ -1,5 +1,5 @@
+/* This file is a hack to have something more reliable than the upstream `common` tag
+  that is implicitly generated as the common chunk through webpack's `splitChunks` config */
+
 import './public-path';
 import 'font-awesome/css/font-awesome.css';
-import 'styles/application.scss';
-
-require.context('../images/', true);

app/javascript/skins/glitch/default/common.scss

@@ -0,0 +1 @@
+@import 'flavours/glitch/styles/index';

app/javascript/skins/vanilla/default/common.scss

@@ -0,0 +1 @@
+@import 'styles/application';

app/lib/themes.rb

@@ -7,7 +7,8 @@ class Themes
   include Singleton
 
   def initialize
-    result = {}
+    @flavours = {}
+
     Rails.root.glob('app/javascript/flavours/*/theme.yml') do |pathname|
       data = YAML.load_file(pathname)
       next unless data['pack']
@@ -35,42 +36,34 @@ class Themes
       data['name'] = name
       data['locales'] = locales
       data['screenshot'] = screenshots
-      data['skin'] = { 'default' => [] }
+      data['skins'] = []
-      result[name] = data
+      @flavours[name] = data
     end
 
     Rails.root.glob('app/javascript/skins/*/*') do |pathname|
       ext = pathname.extname.to_s
       skin = pathname.basename.to_s
       name = pathname.dirname.basename.to_s
-      next unless result[name]
+      next unless @flavours[name]
 
       if pathname.directory?
-        pack = []
+        @flavours[name]['skins'] << skin if pathname.glob('{common,index,application}.{css,scss}').any?
-        pathname.glob('*.{css,scss}') do |sheet|
-          pack.push(sheet.basename(sheet.extname).to_s)
-        end
       elsif /^\.s?css$/i.match?(ext)
-        skin = pathname.basename(ext).to_s
+        @flavours[name]['skins'] << pathname.basename(ext).to_s
-        pack = ['common']
       end
-
-      result[name]['skin'][skin] = pack if skin != 'default'
     end
-
-    @conf = result
   end
 
   def flavour(name)
-    @conf[name]
+    @flavours[name]
   end
 
   def flavours
-    @conf.keys
+    @flavours.keys
   end
 
   def skins_for(name)
-    @conf[name]['skin'].keys
+    @flavours[name]['skins']
   end
 
   def flavours_and_skins

app/views/layouts/_theme.html.haml

@@ -1,12 +1,5 @@
-- if theme
+- if theme && theme[:pack]
-  = render partial: 'layouts/theme', object: theme[:common] if theme[:pack] != 'common' && theme[:common]
+  - pack_path = "flavours/#{theme[:flavour]}/#{theme[:pack]}"
-  - if theme[:pack]
+  = javascript_pack_tag pack_path, crossorigin: 'anonymous'
-    - pack_path = "flavours/#{theme[:flavour]}/#{theme[:pack]}"
+  - theme[:preload]&.each do |link|
-    = javascript_pack_tag pack_path, crossorigin: 'anonymous'
+    %link{ href: asset_pack_path("#{link}.js"), crossorigin: 'anonymous', rel: 'preload', as: 'script' }/
-    - if theme[:skin]
-      - if !theme[:flavour] || theme[:skin] == 'default'
-        = stylesheet_pack_tag pack_path, media: 'all', crossorigin: 'anonymous'
-      - else
-        = stylesheet_pack_tag "skins/#{theme[:flavour]}/#{theme[:skin]}/#{theme[:pack]}", media: 'all', crossorigin: 'anonymous'
-    - theme[:preload]&.each do |link|
-      %link{ href: asset_pack_path("#{link}.js"), crossorigin: 'anonymous', rel: 'preload', as: 'script' }/

app/views/layouts/application.html.haml

@@ -26,11 +26,13 @@
 
     %title= html_title
 
-    = javascript_pack_tag 'common', crossorigin: 'anonymous'
+    = stylesheet_pack_tag 'flavours/vanilla/common', media: 'all', crossorigin: 'anonymous' # upstream uses `common` but that's implicitly defined
+    = stylesheet_pack_tag current_theme, media: 'all', crossorigin: 'anonymous'
 
     -# Needed for the wicg-inert polyfill. It needs to be on it's own <style> tag, with this `id`
     = stylesheet_pack_tag 'flavours/vanilla/inert', media: 'all', id: 'inert-style'
 
+    = javascript_pack_tag 'common', crossorigin: 'anonymous'
     - if @theme
       - if @theme[:supported_locales].include? I18n.locale.to_s
         = preload_pack_asset "locales/#{@theme[:flavour]}/#{I18n.locale}-json.js"
@@ -39,13 +41,13 @@
     = csrf_meta_tags unless skip_csrf_meta_tags?
     %meta{ name: 'style-nonce', content: request.content_security_policy_nonce }
 
+    = stylesheet_link_tag custom_css_path, skip_pipeline: true, host: root_url, media: 'all'
+
     = yield :header_tags
 
     -# These must come after :header_tags to ensure our initial state has been defined.
     = render partial: 'layouts/theme', object: @theme
 
-    = stylesheet_link_tag custom_css_path, skip_pipeline: true, host: root_url, media: 'all'
-
   %body{ class: body_classes }
     = content_for?(:content) ? yield(:content) : yield
 

app/views/layouts/embedded.html.haml

@@ -11,13 +11,16 @@
     - if storage_host?
       %link{ rel: 'dns-prefetch', href: storage_host }/
 
-    = render_initial_state
+    = stylesheet_pack_tag 'flavours/vanilla/common', media: 'all', crossorigin: 'anonymous' # upstream uses `common` but that's implicitly defined
-    = javascript_pack_tag 'common', crossorigin: 'anonymous'
+    = stylesheet_pack_tag current_theme, media: 'all', crossorigin: 'anonymous'
+    = javascript_pack_tag 'common', integrity: true, crossorigin: 'anonymous'
     - if @theme
       - if @theme[:supported_locales].include? I18n.locale.to_s
         = preload_pack_asset "locales/#{@theme[:flavour]}/#{I18n.locale}-json.js"
       - elsif @theme[:supported_locales].include? 'en'
         = preload_pack_asset "locales/#{@theme[:flavour]}/en-json.js"
+    = preload_pack_asset "locale/#{I18n.locale}-json.js"
+    = render_initial_state
     = render partial: 'layouts/theme', object: @theme
 
   %body.embed

app/views/layouts/error.html.haml

@@ -5,6 +5,8 @@
     %meta{ charset: 'utf-8' }/
     %title= safe_join([yield(:page_title), Setting.default_settings['site_title']], ' - ')
     %meta{ content: 'width=device-width,initial-scale=1', name: 'viewport' }/
+    = stylesheet_pack_tag 'flavours/vanilla/common', media: 'all', crossorigin: 'anonymous' # upstream uses `common` but that's implicitly defined
+    = stylesheet_pack_tag current_theme, media: 'all', crossorigin: 'anonymous'
     = javascript_pack_tag 'common', crossorigin: 'anonymous'
     = render partial: 'layouts/theme', object: @theme || { pack: 'error', flavour: 'glitch', common: { pack: 'common', flavour: 'glitch', skin: 'default' } }
   %body.error

config/webpack/configuration.js

@@ -1,7 +1,7 @@
 // Common configuration for webpacker loaded from config/webpacker.yml
 
 const { lstatSync, readFileSync } = require('fs');
-const { basename, dirname, extname, join, resolve } = require('path');
+const { basename, dirname, join, resolve } = require('path');
 const { env } = require('process');
 
 const glob = require('glob');
@@ -37,12 +37,13 @@ skinFiles.forEach((skinFile) => {
   const data = flavours[name].skin;
   if (lstatSync(skinFile).isDirectory()) {
     data[skin] = {};
-    const skinPacks = glob.sync(join(skinFile, '*.{css,scss}'));
+    // TODO: more cleanly take the first match
+    const skinPacks = glob.sync(join(skinFile, '{common,index,application}.{css,scss}'));
     skinPacks.forEach((pack) => {
-      data[skin][basename(pack, extname(pack))] = pack;
+      data[skin] = pack;
     });
   } else if ((skin = skin.match(/^(.*)\.s?css$/i))) {
-    data[skin[1]] = { common: skinFile };
+    data[skin[1]] = skinFile;
   }
 });
 

config/webpack/shared.js

@@ -36,12 +36,7 @@ function reducePacks (data, into = {}) {
     const skin = data.skin[skinName];
     if (!skin) continue;
 
-    for (const entry in skin) {
+    into[`skins/${data.name}/${skinName}`] = resolve(skin);
-      const packFile = skin[entry];
-      if (!packFile) continue;
-
-      into[`skins/${data.name}/${skinName}/${entry}`] = resolve(packFile);
-    }
   }
 
   return into;

lib/tasks/assets.rake

@@ -8,6 +8,14 @@ namespace :assets do
         def current_user
           nil
         end
+
+        def current_flavour
+          Setting.default_settings['flavour']
+        end
+
+        def current_skin
+          Setting.default_settings['skin']
+        end
       end
 
       html = renderer.render(action, opts)