DEV: Do not manipulate theme module paths at build-time (#23148)

Manipulating theme module paths means that the paths you author are not the ones used at runtime. This can lead to some very unexpected behavior and potential module name clashes. It also meant that the refactor in 16c6ab8661 was unable to correctly match up theme connector js/templates.

While this could technically be a breaking change, I think it is reasonably safe because:

1. Themes are already forced to use relative paths when referencing their own modules (since they're namespaced based on the site-specific id). The only time this might be problematic is when theme tests reference modules in the theme's main `javascripts` directory

2. For things like components/services/controllers/etc. our custom Ember resolver works backwards from the end of the path, so adding `discourse/` in the middle will not affect resolution.
This commit is contained in:
David Taylor 2023-08-18 18:15:23 +01:00 committed by GitHub
parent 291749e35b
commit 82b16f4f47
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
5 changed files with 9 additions and 7 deletions

View File

@ -6,7 +6,7 @@ require "json_schemer"
class Theme < ActiveRecord::Base
include GlobalPath
BASE_COMPILER_VERSION = 72
BASE_COMPILER_VERSION = 73
attr_accessor :child_components

View File

@ -234,7 +234,7 @@ class ThemeJavascriptCompiler
def append_module(script, name, include_variables: true)
original_filename = name
name = "discourse/theme-#{@theme_id}/#{name.gsub(%r{\Adiscourse/}, "")}"
name = "discourse/theme-#{@theme_id}/#{name}"
script = "#{theme_settings}#{script}" if include_variables
transpiler = DiscourseJsProcessor::Transpiler.new

View File

@ -119,7 +119,9 @@ RSpec.describe ThemeJavascriptCompiler do
"discourse/templates/components/mycomponent.hbs" => "{{my-component-template}}",
},
)
expect(compiler.raw_content).to include('define("discourse/theme-1/components/mycomponent"')
expect(compiler.raw_content).to include(
'define("discourse/theme-1/discourse/components/mycomponent"',
)
expect(compiler.raw_content).to include(
'define("discourse/theme-1/discourse/templates/components/mycomponent"',
)

View File

@ -259,10 +259,10 @@ HTML
)
expect(theme.javascript_cache.content).to include('addRawTemplate("discovery"')
expect(theme.javascript_cache.content).to include(
"define(\"discourse/theme-#{theme.id}/controllers/discovery\"",
"define(\"discourse/theme-#{theme.id}/discourse/controllers/discovery\"",
)
expect(theme.javascript_cache.content).to include(
"define(\"discourse/theme-#{theme.id}/controllers/discovery-2\"",
"define(\"discourse/theme-#{theme.id}/discourse/controllers/discovery-2\"",
)
expect(theme.javascript_cache.content).to include("const settings =")
expect(theme.javascript_cache.content).to include(

View File

@ -254,7 +254,7 @@ HTML
expect(javascript_cache.content).to include("if ('define' in window) {")
expect(javascript_cache.content).to include(
"define(\"discourse/theme-#{field.theme_id}/initializers/theme-field-#{field.id}-mobile-html-script-1\"",
"define(\"discourse/theme-#{field.theme_id}/discourse/initializers/theme-field-#{field.id}-mobile-html-script-1\"",
)
expect(javascript_cache.content).to include(
"settings = require(\"discourse/lib/theme-settings-store\").getObjectForTheme(#{field.theme_id});",
@ -406,7 +406,7 @@ HTML
)
expect(theme_field.javascript_cache.content).to include("if ('define' in window) {")
expect(theme_field.javascript_cache.content).to include(
"define(\"discourse/theme-#{theme_field.theme.id}/initializers/theme-field-#{theme_field.id}-common-html-script-1\",",
"define(\"discourse/theme-#{theme_field.theme.id}/discourse/initializers/theme-field-#{theme_field.id}-common-html-script-1\",",
)
expect(theme_field.javascript_cache.content).to include(
"name: \"theme-field-#{theme_field.id}-common-html-script-1\",",