DEV: Remove USE_EMBROIDER flag (#23971)

Embroider has been the default since b72ed3cb38. This commit removes the ability to set `USE_EMBROIDER=0` and go back to the classic build.

.github/workflows/tests.yml

@@ -22,7 +22,7 @@ permissions:
 jobs:
   build:
     if: github.event_name == 'pull_request' || github.repository != 'discourse/discourse-private-mirror'
-    name: ${{ matrix.target }} ${{ matrix.build_type }} ${{ matrix.ruby }}${{(matrix.embroider == '1') && ' (Embroider)' || ''}}
+    name: ${{ matrix.target }} ${{ matrix.build_type }} ${{ matrix.ruby }}
     runs-on: ${{ (matrix.build_type == 'annotations') && 'ubuntu-latest' || 'ubuntu-20.04-8core' }}
     container: discourse/discourse_test:slim${{ (matrix.build_type == 'frontend' || matrix.build_type == 'system') && '-browsers' || '' }}${{ (matrix.ruby == '3.1') && '-ruby-3.1.0' || '' }}
     timeout-minutes: 20
@@ -32,7 +32,6 @@ jobs:
       RAILS_ENV: test
       PGUSER: discourse
       PGPASSWORD: discourse
-      USE_EMBROIDER: ${{ matrix.embroider }}
       USES_PARALLEL_DATABASES: ${{ matrix.build_type == 'backend' || matrix.build_type == 'system' }}
       CAPYBARA_DEFAULT_MAX_WAIT_TIME: 10
       MINIO_RUNNER_LOG_LEVEL: DEBUG
@@ -42,18 +41,13 @@ jobs:
 
       matrix:
         build_type: [backend, frontend, system, annotations]
-        embroider: ["0", "1"]
         target: [core, plugins]
         ruby: ["3.2"]
         exclude:
-          - build_type: annotations
-            embroider: "1"
           - build_type: annotations
             target: plugins
           - build_type: frontend
             target: core # Handled by core_frontend_tests job (below)
-          - build_type: backend
-            embroider: "1"
 
     steps:
       - name: Set working directory owner
@@ -219,7 +213,7 @@ jobs:
       - uses: actions/upload-artifact@v3
         if: always() && matrix.build_type == 'frontend' && matrix.target == 'plugins'
         with:
-          name: ember-exam-execution-plugins-frontend-${{(matrix.embroider == '1') && 'embroider' || 'classic'}}
+          name: ember-exam-execution-plugins-frontend
           path: ./app/assets/javascripts/discourse/test-execution-*.json
 
       - name: Ember Build for System Tests
@@ -261,7 +255,7 @@ jobs:
 
   core_frontend_tests:
     if: github.event_name == 'pull_request' || github.repository != 'discourse/discourse-private-mirror'
-    name: core frontend (${{ matrix.browser }})${{(matrix.embroider == '1') && ' (Embroider)' || ''}}
+    name: core frontend (${{ matrix.browser }})
     runs-on: ubuntu-20.04-8core
     container:
       image: discourse/discourse_test:slim-browsers
@@ -272,17 +266,9 @@ jobs:
     strategy:
       fail-fast: false
       matrix:
-        embroider: ["1", "0"]
         browser: ["Chrome", "Firefox ESR", "Firefox Evergreen"]
-        exclude:
-          # Testing the classic build on one browser is good enough
-          - embroider: "0"
-            browser: Firefox ESR
-          - embroider: "0"
-            browser: Firefox Evergreen
 
     env:
-      USE_EMBROIDER: ${{ matrix.embroider }}
       TESTEM_BROWSER: ${{ (startsWith(matrix.browser, 'Firefox') && 'Firefox') || matrix.browser }}
       TESTEM_FIREFOX_PATH: ${{ (matrix.browser == 'Firefox Evergreen') && '/opt/firefox-evergreen/firefox' }}
 

app/assets/javascripts/discourse/ember-cli-build.js

@@ -11,6 +11,8 @@ const generateScriptsTree = require("./lib/scripts");
 const funnel = require("broccoli-funnel");
 const DeprecationSilencer = require("deprecation-silencer");
 const generateWorkboxTree = require("./lib/workbox-tree-builder");
+const { compatBuild } = require("@embroider/compat");
+const { Webpack } = require("@embroider/webpack");
 
 process.env.BROCCOLI_ENABLED_MEMOIZE = true;
 
@@ -22,28 +24,8 @@ module.exports = function (defaults) {
   DeprecationSilencer.silence(console, "warn");
   DeprecationSilencer.silence(defaults.project.ui, "writeWarnLine");
 
-  const isEmbroider = process.env.USE_EMBROIDER !== "0";
   const isProduction = EmberApp.env().includes("production");
 
-  // This is more or less the same as the one in @embroider/test-setup
-  const maybeEmbroider = (app, options) => {
-    if (isEmbroider) {
-      const { compatBuild } = require("@embroider/compat");
-      const { Webpack } = require("@embroider/webpack");
-
-      // https://github.com/embroider-build/embroider/issues/1581
-      if (Array.isArray(options?.extraPublicTrees)) {
-        options.extraPublicTrees = [
-          app.addonPostprocessTree("all", mergeTrees(options.extraPublicTrees)),
-        ];
-      }
-
-      return compatBuild(app, Webpack, options);
-    } else {
-      return app.toTree(options?.extraPublicTrees);
-    }
-  };
-
   const app = new EmberApp(defaults, {
     autoRun: false,
     "ember-qunit": {
@@ -56,40 +38,13 @@ module.exports = function (defaults) {
       enabled: true,
     },
     autoImport: {
-      forbidEval: true,
+      // TODO: Ideally we shouldn't be relying on autoImport at all, but this tweak is still necessary for script/check_reproducible_assets.rb to pass
-      insertScriptsAt: "ember-auto-import-scripts",
+      // Sounds like it's related to the `app.addonPostprocessTree` workaround we use below. Once that's removed, we should be
-      watchDependencies: ["discourse-i18n"],
+      // able to remove this.
       webpack: {
-        // Workarounds for https://github.com/ef4/ember-auto-import/issues/519 and https://github.com/ef4/ember-auto-import/issues/478
-        devtool: isProduction ? false : "source-map", // Sourcemaps contain reference to the ephemeral broccoli cache dir, which changes on every deploy
         optimization: {
           moduleIds: "size", // Consistent module references https://github.com/ef4/ember-auto-import/issues/478#issuecomment-1000526638
         },
-        resolve: {
-          fallback: {
-            // Sinon needs a `util` polyfill
-            util: require.resolve("util/"),
-            // Also for sinon
-            timers: false,
-          },
-        },
-        module: {
-          rules: [
-            // Sinon/`util` polyfill accesses the `process` global,
-            // so we need to provide a mock
-            {
-              test: require.resolve("util/"),
-              use: [
-                {
-                  loader: "imports-loader",
-                  options: {
-                    additionalCode: "var process = { env: {} };",
-                  },
-                },
-              ],
-            },
-          ],
-        },
       },
     },
     fingerprint: {
@@ -107,9 +62,7 @@ module.exports = function (defaults) {
 
     "ember-cli-terser": {
       enabled: isProduction,
-      exclude:
+      exclude: ["**/highlightjs/*", "**/javascripts/*"],
-        ["**/highlightjs/*", "**/javascripts/*"] +
-        (isEmbroider ? [] : ["**/test-*.js", "**/core-tests*.js"]),
     },
 
     "ember-cli-babel": {
@@ -120,10 +73,6 @@ module.exports = function (defaults) {
       plugins: [require.resolve("deprecation-silencer")],
     },
 
-    // Was previously true so that we could run theme tests in production
-    // but we're moving away from that as part of the Embroider migration
-    tests: isEmbroider ? !isProduction : true,
-
     vendorFiles: {
       // Freedom patch - includes bug fix and async stack support
       // https://github.com/discourse/backburner.js/commits/discourse-patches
@@ -165,7 +114,7 @@ module.exports = function (defaults) {
   ]);
   app.project.liveReloadFilterPatterns = [/.*\.scss/];
 
-  const extraPublicTrees = [
+  let extraPublicTrees = [
     createI18nTree(discourseRoot, vendorJs),
     parsePluginClientSettings(discourseRoot, vendorJs, app),
     funnel(`${discourseRoot}/public/javascripts`, { destDir: "javascripts" }),
@@ -191,7 +140,12 @@ module.exports = function (defaults) {
     testStylesheetTree,
   ];
 
-  return maybeEmbroider(app, {
+  // https://github.com/embroider-build/embroider/issues/1581
+  extraPublicTrees = [
+    app.addonPostprocessTree("all", mergeTrees(extraPublicTrees)),
+  ];
+
+  return compatBuild(app, Webpack, {
     extraPublicTrees,
     packagerOptions: {
       webpackConfig: {

app/assets/javascripts/discourse/tests/index.html

@@ -57,7 +57,6 @@
     <discourse-chunked-script entrypoint="discourse-for-tests">
       <ember-auto-import-scripts entrypoint="app"></ember-auto-import-scripts>
       <script src="{{rootURL}}assets/discourse.js"></script>
-      <script defer src="{{rootURL}}assets/tests.js" data-embroider-ignore></script> <!-- Will 404 under embroider. Can be removed once we drop legacy build. -->
     </discourse-chunked-script>
 
     <script src="{{rootURL}}assets/test-i18n.js" data-embroider-ignore></script>