From c285f4133fd33b2eaf107b776f8d28aaf35beb8f Mon Sep 17 00:00:00 2001 From: Jarek Radosz Date: Tue, 5 Sep 2023 11:16:12 +0200 Subject: [PATCH] DEV: Enable ember-this-fallback in themes (#23384) Co-authored-by: David Taylor --- .../javascripts/discourse-plugins/index.js | 4 + .../discourse/config/deprecation-workflow.js | 1 - app/assets/javascripts/js-processor.js | 11 +- .../patches/ember-this-fallback+0.3.1.patch | 123 +++++++++++++++++- spec/lib/theme_javascript_compiler_spec.rb | 26 +++- spec/models/theme_field_spec.rb | 2 +- 6 files changed, 160 insertions(+), 7 deletions(-) diff --git a/app/assets/javascripts/discourse-plugins/index.js b/app/assets/javascripts/discourse-plugins/index.js index 71ea720dca8..65d6daa821b 100644 --- a/app/assets/javascripts/discourse-plugins/index.js +++ b/app/assets/javascripts/discourse-plugins/index.js @@ -100,6 +100,10 @@ module.exports = { plugins: [require.resolve("deprecation-silencer")], }, + "ember-cli-babel": { + throwUnlessParallelizable: true, + }, + "ember-this-fallback": { enableLogging: false, }, diff --git a/app/assets/javascripts/discourse/config/deprecation-workflow.js b/app/assets/javascripts/discourse/config/deprecation-workflow.js index 9fbeb0e2991..6beb637f2b4 100644 --- a/app/assets/javascripts/discourse/config/deprecation-workflow.js +++ b/app/assets/javascripts/discourse/config/deprecation-workflow.js @@ -5,7 +5,6 @@ globalThis.deprecationWorkflow.config = { workflow: [ { handler: "silence", matchId: "route-render-template" }, { handler: "silence", matchId: "route-disconnect-outlet" }, - { handler: "silence", matchId: "this-property-fallback" }, // We can unsilence this once ember-this-fallback works with themes { handler: "silence", matchId: "ember-this-fallback.this-property-fallback", diff --git a/app/assets/javascripts/js-processor.js b/app/assets/javascripts/js-processor.js index 093306ab073..3beeb806b1b 100644 --- a/app/assets/javascripts/js-processor.js +++ b/app/assets/javascripts/js-processor.js @@ -24,6 +24,12 @@ import { transform as babelTransform } from "@babel/standalone"; import { minify as terserMinify } from "terser"; import RawHandlebars from "discourse-common/addon/lib/raw-handlebars"; import { WidgetHbsCompiler } from "discourse-widget-hbs/lib/widget-hbs-compiler"; +import EmberThisFallback from "ember-this-fallback"; + +const thisFallbackPlugin = EmberThisFallback._buildPlugin({ + enableLogging: false, + isTheme: true, +}).plugin; function manipulateAstNodeForTheme(node, themeId) { // Magically add theme id as the first param for each of these helpers) @@ -62,7 +68,10 @@ function buildTemplateCompilerBabelPlugins({ themeId }) { return precompile(src, { ...opts, plugins: { - ast: [buildEmberTemplateManipulatorPlugin(themeId)], + ast: [ + buildEmberTemplateManipulatorPlugin(themeId), + thisFallbackPlugin, + ], }, }); }; diff --git a/app/assets/javascripts/patches/ember-this-fallback+0.3.1.patch b/app/assets/javascripts/patches/ember-this-fallback+0.3.1.patch index 272727fe992..4aa1db599d5 100644 --- a/app/assets/javascripts/patches/ember-this-fallback+0.3.1.patch +++ b/app/assets/javascripts/patches/ember-this-fallback+0.3.1.patch @@ -1,5 +1,5 @@ diff --git a/node_modules/ember-this-fallback/index.js b/node_modules/ember-this-fallback/index.js -index afa73eb..168bb16 100644 +index afa73eb..21e9411 100644 --- a/node_modules/ember-this-fallback/index.js +++ b/node_modules/ember-this-fallback/index.js @@ -8,7 +8,7 @@ module.exports = { @@ -11,6 +11,32 @@ index afa73eb..168bb16 100644 registry.add('htmlbars-ast-plugin', this._buildPlugin(options)); } }, +@@ -17,15 +17,20 @@ module.exports = { + ThisFallbackPlugin.baseDir = () => __dirname; + ThisFallbackPlugin.cacheKey = () => 'ember-this-fallback'; + +- return { ++ let plugin = { + name: 'ember-this-fallback', +- parallelBabel: { ++ plugin: ThisFallbackPlugin(options), ++ }; ++ ++ if (!options.isTheme) { ++ plugin.parallelBabel = { + requireFile: __filename, + buildUsing: '_buildPlugin', + params: options, +- }, +- plugin: ThisFallbackPlugin(options), +- }; ++ }; ++ } ++ ++ return plugin; + }, + }; + diff --git a/node_modules/ember-this-fallback/lib/helpers/deprecations.js b/node_modules/ember-this-fallback/lib/helpers/deprecations.js index 5a00c44..993e3cf 100644 --- a/node_modules/ember-this-fallback/lib/helpers/deprecations.js @@ -38,3 +64,98 @@ index 5a00c44..993e3cf 100644 function deprecationOptionsFor(id) { const options = DEPRECATION_OPTIONS_MAP.get(id); (0, assert_1.default)(`expected to find DeprecationOptions for id=${id}`, options); +diff --git a/node_modules/ember-this-fallback/lib/helpers/logger.js b/node_modules/ember-this-fallback/lib/helpers/logger.js +index b856edc..afb059a 100644 +--- a/node_modules/ember-this-fallback/lib/helpers/logger.js ++++ b/node_modules/ember-this-fallback/lib/helpers/logger.js +@@ -1,12 +1,6 @@ + "use strict"; +-var __importDefault = (this && this.__importDefault) || function (mod) { +- return (mod && mod.__esModule) ? mod : { "default": mod }; +-}; + Object.defineProperty(exports, "__esModule", { value: true }); + exports.noopLogger = void 0; +-const debug_1 = __importDefault(require("debug")); +-const winston_1 = require("winston"); +-const winston_transport_1 = __importDefault(require("winston-transport")); + function noopLogger() { + return { + debug: noop, +@@ -15,53 +9,4 @@ function noopLogger() { + }; + } + exports.noopLogger = noopLogger; +-function createLogger(namespace, label) { +- const debug = (0, debug_1.default)(namespace); +- class DebugTransport extends winston_transport_1.default { +- log(info, next) { +- debug(info[Symbol.for('message')]); +- next(); +- } +- } +- return (0, winston_1.createLogger)({ +- level: 'debug', +- transports: [ +- new winston_1.transports.File({ +- level: 'info', +- filename: `${namespace}.log`, +- format: winston_1.format.combine(joinLines(), winston_1.format.label({ label }), winston_1.format.timestamp(), winston_1.format.splat(), logFormatter), +- }), +- new DebugTransport({ +- level: 'debug', +- format: winston_1.format.combine(joinLines(), winston_1.format.label({ label }), winston_1.format.timestamp(), winston_1.format.splat(), debugFormatter), +- }), +- ], +- }); +-} +-exports.default = createLogger; +-const joinLines = (0, winston_1.format)((info) => { +- if (Array.isArray(info.message) && +- info.message.every((m) => typeof m === 'string')) { +- info.message = joinLogLines(info.message); +- } +- return info; +-}); +-const logFormatter = winston_1.format.printf((info) => { +- const { level, label, timestamp, message, loc } = info; +- return `${timestamp} [${level}] ${concatMessage(label, message, loc)}`; +-}); +-const debugFormatter = winston_1.format.printf((info) => { +- const { label, message } = info; +- return concatMessage(label, message); +-}); +-function concatMessage(label, message, loc) { +- if (loc) { +- const start = loc.startPosition; +- label += `:${start.line}:${start.column + 1}`; +- } +- return joinLogLines([label, message]); +-} +-function joinLogLines(lines) { +- return lines.join('\n\t'); +-} + function noop() { } +diff --git a/node_modules/ember-this-fallback/lib/this-fallback-plugin.js b/node_modules/ember-this-fallback/lib/this-fallback-plugin.js +index be4a785..a8ee337 100644 +--- a/node_modules/ember-this-fallback/lib/this-fallback-plugin.js ++++ b/node_modules/ember-this-fallback/lib/this-fallback-plugin.js +@@ -29,7 +29,7 @@ const syntax_1 = require("@glimmer/syntax"); + const ast_1 = require("./helpers/ast"); + const deprecations_1 = require("./helpers/deprecations"); + const fallback_1 = require("./helpers/fallback"); +-const logger_1 = __importStar(require("./helpers/logger")); ++const logger_1 = require("./helpers/logger"); + const scope_stack_1 = __importStar(require("./helpers/scope-stack")); + const string_1 = require("./helpers/string"); + const assert_1 = __importDefault(require("./types/assert")); +@@ -214,9 +214,7 @@ class NoopPlugin { + function buildThisFallbackPlugin({ enableLogging, }) { + return (env) => { + const name = 'ember-this-fallback'; +- const logger = enableLogging +- ? (0, logger_1.default)(`${name}-plugin`, env.moduleName) +- : (0, logger_1.noopLogger)(); ++ const logger = (0, logger_1.noopLogger)(); + // eslint-disable-next-line @typescript-eslint/no-unnecessary-condition + if (env.meta.jsutils) { + return new ThisFallbackPlugin(name, env, logger); diff --git a/spec/lib/theme_javascript_compiler_spec.rb b/spec/lib/theme_javascript_compiler_spec.rb index abfa0c0e899..85aeab5ffe5 100644 --- a/spec/lib/theme_javascript_compiler_spec.rb +++ b/spec/lib/theme_javascript_compiler_spec.rb @@ -24,17 +24,17 @@ RSpec.describe ThemeJavascriptCompiler do it "maintains module names so that discourse-boot.js can correct them" do compiler.append_ember_template("/connectors/blah-1", "{{var}}") expect(compiler.raw_content.to_s).to include( - "define(\"discourse/theme-1/connectors/blah-1\", [\"exports\", \"@ember/template-factory\"]", + "define(\"discourse/theme-1/connectors/blah-1\", [\"exports\", ", ) compiler.append_ember_template("connectors/blah-2", "{{var}}") expect(compiler.raw_content.to_s).to include( - "define(\"discourse/theme-1/connectors/blah-2\", [\"exports\", \"@ember/template-factory\"]", + "define(\"discourse/theme-1/connectors/blah-2\", [\"exports\", ", ) compiler.append_ember_template("javascripts/connectors/blah-3", "{{var}}") expect(compiler.raw_content.to_s).to include( - "define(\"discourse/theme-1/javascripts/connectors/blah-3\", [\"exports\", \"@ember/template-factory\"]", + "define(\"discourse/theme-1/javascripts/connectors/blah-3\", [\"exports\", ", ) end end @@ -213,4 +213,24 @@ RSpec.describe ThemeJavascriptCompiler do expect(compiler.content).to include("Unexpected token") end end + + describe "ember-this-fallback" do + it "applies its transforms" do + compiler.append_tree( + { + "discourse/components/my-component.js" => <<~JS, + import Component from "@glimmer/component"; + export default class MyComponent extends Component { + value = "foo"; + } + JS + "discourse/components/my-component.hbs" => "{{value}}", + }, + ) + expect(compiler.raw_content).to include("ember-this-fallback") + expect(compiler.raw_content).to include( + "The `value` property path was used in the `discourse/components/my-component.hbs` template without using `this`. This fallback behavior has been deprecated, all properties must be looked up on `this` when used in the template: {{this.value}}", + ) + end + end end diff --git a/spec/models/theme_field_spec.rb b/spec/models/theme_field_spec.rb index a34ea1600f1..8e2260c903e 100644 --- a/spec/models/theme_field_spec.rb +++ b/spec/models/theme_field_spec.rb @@ -255,7 +255,7 @@ HTML # All together expect(theme.javascript_cache.content).to include( - "define(\"discourse/theme-#{theme.id}/discourse/templates/discovery\", [\"exports\", \"@ember/template-factory\"]", + "define(\"discourse/theme-#{theme.id}/discourse/templates/discovery\", [\"exports\", ", ) expect(theme.javascript_cache.content).to include('addRawTemplate("discovery"') expect(theme.javascript_cache.content).to include(