From 8c01947c45e1cb3da2d1ae06f21e41180f9c5ebe Mon Sep 17 00:00:00 2001 From: David Taylor Date: Thu, 19 Oct 2023 10:38:25 +0100 Subject: [PATCH] DEV: Remove USE_EMBROIDER flag (#23971) Embroider has been the default since b72ed3cb38d58d4cf316f39d7e9de0e722729b5b. This commit removes the ability to set `USE_EMBROIDER=0` and go back to the classic build. --- .github/workflows/tests.yml | 20 +----- .../javascripts/discourse/ember-cli-build.js | 72 ++++--------------- .../javascripts/discourse/tests/index.html | 1 - 3 files changed, 16 insertions(+), 77 deletions(-) diff --git a/.github/workflows/tests.yml b/.github/workflows/tests.yml index d361c4a5977..96c35cddf7e 100644 --- a/.github/workflows/tests.yml +++ b/.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' }} diff --git a/app/assets/javascripts/discourse/ember-cli-build.js b/app/assets/javascripts/discourse/ember-cli-build.js index 26bd558aa58..8d9a6c6fe05 100644 --- a/app/assets/javascripts/discourse/ember-cli-build.js +++ b/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, - insertScriptsAt: "ember-auto-import-scripts", - watchDependencies: ["discourse-i18n"], + // 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 + // Sounds like it's related to the `app.addonPostprocessTree` workaround we use below. Once that's removed, we should be + // 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: - ["**/highlightjs/*", "**/javascripts/*"] + - (isEmbroider ? [] : ["**/test-*.js", "**/core-tests*.js"]), + exclude: ["**/highlightjs/*", "**/javascripts/*"], }, "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: { diff --git a/app/assets/javascripts/discourse/tests/index.html b/app/assets/javascripts/discourse/tests/index.html index 79e334f29dc..0514515915e 100644 --- a/app/assets/javascripts/discourse/tests/index.html +++ b/app/assets/javascripts/discourse/tests/index.html @@ -57,7 +57,6 @@ -