From 565c753dd2ddcc6024615551548400c3666edd50 Mon Sep 17 00:00:00 2001 From: David Taylor Date: Mon, 10 Jun 2024 15:51:48 +0100 Subject: [PATCH] DEV: `@babel/plugin-proposal-decorators` -> `decorator-transforms` (#27260) decorator-transforms (https://github.com/ef4/decorator-transforms) is a modern replacement for babel's plugin-proposal-decorators. It provides a decorator implementation using modern browser features, without needing to enable babel's full suite of class feature transformations. This improves the developer experience and performance. In local testing with Google's 'tachometer' tool, this reduces Discourse's 'init-to-render' time by around 3-4% (230ms -> 222ms). It reduces our initial gzip'd JS payloads by 3.2% (2.43MB -> 2.35MB), or 7.5% (14.5MB -> 13.4MB) uncompressed. This was previously reverted in 97847f6. This version includes a babel transformation which works around the bug in Safari <= 15. For Cloudflare compatibility issues, check https://meta.discourse.org/t/311390 --- app/assets/javascripts/admin/index.js | 5 +++ app/assets/javascripts/dialog-holder/index.js | 6 +++ .../javascripts/discourse-common/index.js | 4 ++ .../javascripts/discourse-plugins/index.js | 8 +--- app/assets/javascripts/discourse/app/app.js | 1 + .../javascripts/discourse/ember-cli-build.js | 9 +---- ...babel-plugin-safari-class-fields-bugfix.js | 37 +++++++++++++++++++ .../discourse/lib/common-babel-config.js | 22 +++++++++++ .../discourse/tests/setup-tests.js | 2 +- app/assets/javascripts/float-kit/index.js | 6 ++- app/assets/javascripts/pretty-text/index.js | 6 +++ app/assets/javascripts/select-kit/index.js | 6 ++- .../javascripts/theme-transpiler/package.json | 3 +- .../theme-transpiler/transpiler.js | 5 +++ app/models/theme.rb | 2 +- lib/discourse_js_processor.rb | 4 +- patches/decorator-transforms+2.0.0.patch | 21 +++++++++++ spec/lib/discourse_js_processor_spec.rb | 2 +- spec/lib/theme_javascript_compiler_spec.rb | 16 +++++++- 19 files changed, 141 insertions(+), 24 deletions(-) create mode 100644 app/assets/javascripts/discourse/lib/babel-plugin-safari-class-fields-bugfix.js create mode 100644 app/assets/javascripts/discourse/lib/common-babel-config.js create mode 100644 patches/decorator-transforms+2.0.0.patch diff --git a/app/assets/javascripts/admin/index.js b/app/assets/javascripts/admin/index.js index 8889660fdb5..c0930313d93 100644 --- a/app/assets/javascripts/admin/index.js +++ b/app/assets/javascripts/admin/index.js @@ -2,10 +2,15 @@ const calculateCacheKeyForTree = require("calculate-cache-key-for-tree"); const path = require("path"); +const commonBabelConfig = require("../discourse/lib/common-babel-config"); module.exports = { name: require("./package").name, + options: { + ...commonBabelConfig(), + }, + // return an empty tree here as we do not want the addon modules to be // included into vendor.js; instead, we will produce a separate bundle // (admin.js) to be included via a script tag as needed diff --git a/app/assets/javascripts/dialog-holder/index.js b/app/assets/javascripts/dialog-holder/index.js index b5b31da4a49..cbac02da3ea 100644 --- a/app/assets/javascripts/dialog-holder/index.js +++ b/app/assets/javascripts/dialog-holder/index.js @@ -1,8 +1,14 @@ "use strict"; +const commonBabelConfig = require("../discourse/lib/common-babel-config"); + module.exports = { name: require("./package").name, + options: { + ...commonBabelConfig(), + }, + isDevelopingAddon() { return true; }, diff --git a/app/assets/javascripts/discourse-common/index.js b/app/assets/javascripts/discourse-common/index.js index e912e37ba5f..7e8b0dfda18 100644 --- a/app/assets/javascripts/discourse-common/index.js +++ b/app/assets/javascripts/discourse-common/index.js @@ -1,5 +1,7 @@ "use strict"; +const commonBabelConfig = require("../discourse/lib/common-babel-config"); + module.exports = { name: require("./package").name, options: { @@ -8,6 +10,8 @@ module.exports = { handlebars: "handlebars/dist/cjs/handlebars.js", }, }, + + ...commonBabelConfig(), }, isDevelopingAddon() { diff --git a/app/assets/javascripts/discourse-plugins/index.js b/app/assets/javascripts/discourse-plugins/index.js index 851a94da7c4..bb3f2baba34 100644 --- a/app/assets/javascripts/discourse-plugins/index.js +++ b/app/assets/javascripts/discourse-plugins/index.js @@ -96,13 +96,7 @@ module.exports = { name: require("./package").name, options: { - babel: { - plugins: [require.resolve("deprecation-silencer")], - }, - - "ember-cli-babel": { - throwUnlessParallelizable: true, - }, + ...require("../discourse/lib/common-babel-config")(), "ember-this-fallback": { enableLogging: false, diff --git a/app/assets/javascripts/discourse/app/app.js b/app/assets/javascripts/discourse/app/app.js index f3e9e8dc493..d10b152f47e 100644 --- a/app/assets/javascripts/discourse/app/app.js +++ b/app/assets/javascripts/discourse/app/app.js @@ -1,4 +1,5 @@ /* eslint-disable simple-import-sort/imports */ +import "decorator-transforms/globals"; import "./loader-shims"; import "./global-compat"; /* eslint-enable simple-import-sort/imports */ diff --git a/app/assets/javascripts/discourse/ember-cli-build.js b/app/assets/javascripts/discourse/ember-cli-build.js index 94b0fa5681b..e2594ad9106 100644 --- a/app/assets/javascripts/discourse/ember-cli-build.js +++ b/app/assets/javascripts/discourse/ember-cli-build.js @@ -16,6 +16,7 @@ const { StatsWriterPlugin } = require("webpack-stats-plugin"); const withSideWatch = require("./lib/with-side-watch"); const RawHandlebarsCompiler = require("discourse-hbr/raw-handlebars-compiler"); const crypto = require("crypto"); +const commonBabelConfig = require("./lib/common-babel-config"); process.env.BROCCOLI_ENABLED_MEMOIZE = true; @@ -58,13 +59,7 @@ module.exports = function (defaults) { exclude: ["**/highlightjs/*", "**/javascripts/*"], }, - "ember-cli-babel": { - throwUnlessParallelizable: true, - }, - - babel: { - plugins: [require.resolve("deprecation-silencer")], - }, + ...commonBabelConfig(), vendorFiles: { // Freedom patch - includes bug fix and async stack support diff --git a/app/assets/javascripts/discourse/lib/babel-plugin-safari-class-fields-bugfix.js b/app/assets/javascripts/discourse/lib/babel-plugin-safari-class-fields-bugfix.js new file mode 100644 index 00000000000..dadddd6a232 --- /dev/null +++ b/app/assets/javascripts/discourse/lib/babel-plugin-safari-class-fields-bugfix.js @@ -0,0 +1,37 @@ +function wrapInitializer(path, babel) { + const { types: t } = babel; + const { value } = path.node; + + // Check if the node has already been transformed + if (path.node.__wrapped) { + return; + } + + if (value && !(t.isLiteral(value) && !t.isTemplateLiteral(value))) { + path.node.value = t.callExpression( + t.arrowFunctionExpression([], value), + [] + ); + // Mark the node as transformed + path.node.__wrapped = true; + } +} + +function makeVisitor(babel) { + return { + ClassProperty(path) { + wrapInitializer(path, babel); + }, + ClassPrivateProperty(path) { + wrapInitializer(path, babel); + }, + }; +} + +module.exports = function wrapClassFields(babel) { + return { + post(file) { + babel.traverse(file.ast, makeVisitor(babel), file.scope, this); + }, + }; +}; diff --git a/app/assets/javascripts/discourse/lib/common-babel-config.js b/app/assets/javascripts/discourse/lib/common-babel-config.js new file mode 100644 index 00000000000..681e84a0ff8 --- /dev/null +++ b/app/assets/javascripts/discourse/lib/common-babel-config.js @@ -0,0 +1,22 @@ +module.exports = function generateCommonBabelConfig() { + return { + "ember-cli-babel": { + throwUnlessParallelizable: true, + disableDecoratorTransforms: true, + }, + + babel: { + sourceMaps: false, + plugins: [ + require.resolve("deprecation-silencer"), + [ + require.resolve("decorator-transforms"), + { + runEarly: true, + }, + ], + require.resolve("./babel-plugin-safari-class-fields-bugfix"), + ], + }, + }; +}; diff --git a/app/assets/javascripts/discourse/tests/setup-tests.js b/app/assets/javascripts/discourse/tests/setup-tests.js index 5dcc5c144c7..e8bb5686590 100644 --- a/app/assets/javascripts/discourse/tests/setup-tests.js +++ b/app/assets/javascripts/discourse/tests/setup-tests.js @@ -1,4 +1,5 @@ /* eslint-disable simple-import-sort/imports */ +import Application from "../app"; import "./loader-shims"; /* eslint-enable simple-import-sort/imports */ @@ -46,7 +47,6 @@ import deprecated from "discourse-common/lib/deprecated"; import { setDefaultOwner } from "discourse-common/lib/get-owner"; import { setupS3CDN, setupURL } from "discourse-common/lib/get-url"; import { buildResolver } from "discourse-common/resolver"; -import Application from "../app"; import { loadSprites } from "../lib/svg-sprite-loader"; import * as FakerModule from "@faker-js/faker"; import { setLoadedFaker } from "discourse/lib/load-faker"; diff --git a/app/assets/javascripts/float-kit/index.js b/app/assets/javascripts/float-kit/index.js index 28c7dca628f..65b661a2352 100644 --- a/app/assets/javascripts/float-kit/index.js +++ b/app/assets/javascripts/float-kit/index.js @@ -1,8 +1,12 @@ "use strict"; +const commonBabelConfig = require("../discourse/lib/common-babel-config"); + module.exports = { name: require("./package").name, - options: {}, + options: { + ...commonBabelConfig(), + }, isDevelopingAddon() { return true; }, diff --git a/app/assets/javascripts/pretty-text/index.js b/app/assets/javascripts/pretty-text/index.js index b5b31da4a49..cbac02da3ea 100644 --- a/app/assets/javascripts/pretty-text/index.js +++ b/app/assets/javascripts/pretty-text/index.js @@ -1,8 +1,14 @@ "use strict"; +const commonBabelConfig = require("../discourse/lib/common-babel-config"); + module.exports = { name: require("./package").name, + options: { + ...commonBabelConfig(), + }, + isDevelopingAddon() { return true; }, diff --git a/app/assets/javascripts/select-kit/index.js b/app/assets/javascripts/select-kit/index.js index 28c7dca628f..65b661a2352 100644 --- a/app/assets/javascripts/select-kit/index.js +++ b/app/assets/javascripts/select-kit/index.js @@ -1,8 +1,12 @@ "use strict"; +const commonBabelConfig = require("../discourse/lib/common-babel-config"); + module.exports = { name: require("./package").name, - options: {}, + options: { + ...commonBabelConfig(), + }, isDevelopingAddon() { return true; }, diff --git a/app/assets/javascripts/theme-transpiler/package.json b/app/assets/javascripts/theme-transpiler/package.json index 70eeb63979a..b2df0e84b2e 100644 --- a/app/assets/javascripts/theme-transpiler/package.json +++ b/app/assets/javascripts/theme-transpiler/package.json @@ -19,7 +19,8 @@ "handlebars": "^4.7.8", "path-browserify": "^1.0.1", "polyfill-crypto.getrandomvalues": "^1.0.0", - "terser": "^5.31.1" + "terser": "^5.31.1", + "decorator-transforms": "^2.0.0" }, "engines": { "node": ">= 18", diff --git a/app/assets/javascripts/theme-transpiler/transpiler.js b/app/assets/javascripts/theme-transpiler/transpiler.js index 5336a767856..3141289cc75 100644 --- a/app/assets/javascripts/theme-transpiler/transpiler.js +++ b/app/assets/javascripts/theme-transpiler/transpiler.js @@ -19,6 +19,7 @@ globalThis.console = { import { transform as babelTransform } from "@babel/standalone"; import HTMLBarsInlinePrecompile from "babel-plugin-ember-template-compilation"; import { Preprocessor } from "content-tag"; +import DecoratorTransforms from "decorator-transforms"; import colocatedBabelPlugin from "ember-cli-htmlbars/lib/colocated-babel-plugin"; import { precompile } from "ember-source/dist/ember-template-compiler"; import EmberThisFallback from "ember-this-fallback"; @@ -29,6 +30,7 @@ import getRandomValues from "polyfill-crypto.getrandomvalues"; 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 BabelPluginSafariClassFieldsBugfix from "../discourse/lib/babel-plugin-safari-class-fields-bugfix"; globalThis.crypto = { getRandomValues }; const thisFallbackPlugin = EmberThisFallback._buildPlugin({ @@ -138,7 +140,10 @@ globalThis.transpile = function (source, options = {}) { if (moduleId && !skipModule) { plugins.push(["transform-modules-amd", { noInterop: true }]); } + commonPlugins.find((p) => p[0] === "decorator-transforms")[0] = + DecoratorTransforms; plugins.push(...commonPlugins); + plugins.unshift(BabelPluginSafariClassFieldsBugfix); try { return babelTransform(source, { diff --git a/app/models/theme.rb b/app/models/theme.rb index 88f794ad56b..c177d47154c 100644 --- a/app/models/theme.rb +++ b/app/models/theme.rb @@ -6,7 +6,7 @@ require "json_schemer" class Theme < ActiveRecord::Base include GlobalPath - BASE_COMPILER_VERSION = 81 + BASE_COMPILER_VERSION = 84 class SettingsMigrationError < StandardError end diff --git a/lib/discourse_js_processor.rb b/lib/discourse_js_processor.rb index 26d142fa4d8..de807a69642 100644 --- a/lib/discourse_js_processor.rb +++ b/lib/discourse_js_processor.rb @@ -9,9 +9,7 @@ class DiscourseJsProcessor # To generate a list of babel plugins used by ember-cli, set # babel: { debug: true } in ember-cli-build.js, then run `yarn ember build -prod` DISCOURSE_COMMON_BABEL_PLUGINS = [ - ["proposal-decorators", { legacy: true }], - "proposal-class-properties", - "proposal-private-methods", + ["decorator-transforms", { runEarly: true }], "proposal-class-static-block", "transform-parameters", "proposal-export-namespace-from", diff --git a/patches/decorator-transforms+2.0.0.patch b/patches/decorator-transforms+2.0.0.patch new file mode 100644 index 00000000000..6ecd084a303 --- /dev/null +++ b/patches/decorator-transforms+2.0.0.patch @@ -0,0 +1,21 @@ +diff --git a/node_modules/decorator-transforms/dist/index.js b/node_modules/decorator-transforms/dist/index.js +index fce3aeb..c23d8e4 100644 +--- a/node_modules/decorator-transforms/dist/index.js ++++ b/node_modules/decorator-transforms/dist/index.js +@@ -4,10 +4,13 @@ import { + import "./chunk-CSAU5B4Q.js"; + + // src/index.ts +-import { createRequire } from "module"; ++ + import { ImportUtil } from "babel-import-util"; +-var req = createRequire(import.meta.url); +-var { default: decoratorSyntax } = req("@babel/plugin-syntax-decorators"); ++ ++// https://github.com/ef4/decorator-transforms/pull/27 ++import PluginSyntaxDecorators from "@babel/plugin-syntax-decorators"; ++const decoratorSyntax = PluginSyntaxDecorators.default || PluginSyntaxDecorators; ++ + function makeVisitor(babel) { + const t = babel.types; + return { diff --git a/spec/lib/discourse_js_processor_spec.rb b/spec/lib/discourse_js_processor_spec.rb index 5b0142dfbc5..a2f4e506b89 100644 --- a/spec/lib/discourse_js_processor_spec.rb +++ b/spec/lib/discourse_js_processor_spec.rb @@ -92,7 +92,7 @@ RSpec.describe DiscourseJsProcessor do JS result = DiscourseJsProcessor.transpile(script, "blah", "blah/mymodule") - expect(result).to include("_applyDecoratedDescriptor") + expect(result).to include("static #_ = (() => dt7948.n") end it "correctly transpiles widget hbs" do diff --git a/spec/lib/theme_javascript_compiler_spec.rb b/spec/lib/theme_javascript_compiler_spec.rb index 6ad14c5f2fa..dab065bf9db 100644 --- a/spec/lib/theme_javascript_compiler_spec.rb +++ b/spec/lib/theme_javascript_compiler_spec.rb @@ -251,9 +251,23 @@ RSpec.describe ThemeJavascriptCompiler do expect(compiler.raw_content).to include( "define(\"discourse/theme-1/discourse/components/my-component\", [\"exports\",", ) - expect(compiler.raw_content).to include("_defineProperty(this, \"value\", \"foo\");") + expect(compiler.raw_content).to include('value = "foo";') expect(compiler.raw_content).to include("setComponentTemplate") expect(compiler.raw_content).to include("createTemplateFactory") end end + + describe "safari <16 class field bugfix" do + it "is applied" do + compiler.append_tree({ "discourse/components/my-component.js" => <<~JS }) + export default class MyComponent extends Component { + value = "foo"; + complexValue = this.value + "bar"; + } + JS + + expect(compiler.raw_content).to include('value = "foo";') + expect(compiler.raw_content).to include('complexValue = (() => this.value + "bar")();') + end + end end