Bladeren bron

Move wagtailConfig values to wagtail_config template tag

The first step on cleaning up our client-side metadata configuration. With this approach, the values are computed in the `wagtail_config` template tag and passed into the template using Django's json_script.

Then, it's parsed on the client-side and set as `global.wagtailConfig` to retain compatibility with existing code that rely on `window.wagtailConfig`.

This allows us to remove our existing approach of putting metadata values in a `<script>` tag using Django templates directly in the HTML, without changing too much of the existing code, and allowing new code to import the values as `WAGTAIL_CONFIG` from `wagtailConfig.js` instead of using `window.wagtailConfig`. It also means we remove the inline script tag from the core admin base template.

Refactor wagtailConfig util

- Avoid extraneous named exports when they are included in the named WAGTAIL_CONFIG
- Simplify locale map generation
- Avoid reading from global, instead export util that can be used as a global
- Update unit tests for more robust checks
Sage Abdullah 2 jaren geleden
bovenliggende
commit
4bb47f7e25

+ 3 - 1
client/src/api/admin.test.js

@@ -1,7 +1,9 @@
-import { ADMIN_API } from '../config/wagtailConfig';
+import { WAGTAIL_CONFIG } from '../config/wagtailConfig';
 import { getPageChildren, getPage } from './admin';
 import client from './client';
 
+const { ADMIN_API } = WAGTAIL_CONFIG;
+
 jest.mock('./client', () => {
   const stubResult = {
     __types: {

+ 3 - 1
client/src/api/admin.ts

@@ -1,6 +1,8 @@
 import client from './client';
 
-import { ADMIN_API } from '../config/wagtailConfig';
+import { WAGTAIL_CONFIG } from '../config/wagtailConfig';
+
+const { ADMIN_API } = WAGTAIL_CONFIG;
 
 export interface WagtailPageAPI {
   id: number;

+ 3 - 4
client/src/components/ChooserWidget/SnippetChooserWidget.js

@@ -1,17 +1,16 @@
 import { ChooserModal } from '../../includes/chooserModal';
 import { Chooser, ChooserFactory } from '.';
-
-/* global wagtailConfig */
+import { WAGTAIL_CONFIG } from '../../config/wagtailConfig';
 
 class SnippetChooserModal extends ChooserModal {
   getURLParams(opts) {
     const params = super.getURLParams(opts);
-    if (wagtailConfig.ACTIVE_CONTENT_LOCALE) {
+    if (WAGTAIL_CONFIG.ACTIVE_CONTENT_LOCALE) {
       // The user is editing a piece of translated content.
       // Pass the locale along as a request parameter. If this
       // snippet is also translatable, the results will be
       // pre-filtered by this locale.
-      params.locale = wagtailConfig.ACTIVE_CONTENT_LOCALE;
+      params.locale = WAGTAIL_CONFIG.ACTIVE_CONTENT_LOCALE;
     }
     return params;
   }

+ 2 - 2
client/src/components/PageExplorer/PageCount.tsx

@@ -1,7 +1,7 @@
 import React from 'react';
 
 import { gettext } from '../../utils/gettext';
-import { ADMIN_URLS } from '../../config/wagtailConfig';
+import { WAGTAIL_CONFIG } from '../../config/wagtailConfig';
 import Icon from '../Icon/Icon';
 
 interface PageCountProps {
@@ -18,7 +18,7 @@ const PageCount: React.FunctionComponent<PageCountProps> = ({ page }) => {
 
   return (
     <a
-      href={`${ADMIN_URLS.PAGES}${page.id}/`}
+      href={`${WAGTAIL_CONFIG.ADMIN_URLS.PAGES}${page.id}/`}
       className="c-page-explorer__see-more"
     >
       {gettext('See all')}

+ 3 - 1
client/src/components/PageExplorer/PageExplorerHeader.tsx

@@ -1,11 +1,13 @@
 import React from 'react';
-import { ADMIN_URLS } from '../../config/wagtailConfig';
+import { WAGTAIL_CONFIG } from '../../config/wagtailConfig';
 
 import { gettext } from '../../utils/gettext';
 import Link from '../Link/Link';
 import Icon from '../Icon/Icon';
 import { PageState } from './reducers/nodes';
 
+const { ADMIN_URLS } = WAGTAIL_CONFIG;
+
 interface SelectLocaleProps {
   locale?: string;
   translations: Map<string, number>;

+ 3 - 1
client/src/components/PageExplorer/PageExplorerItem.tsx

@@ -1,12 +1,14 @@
 import React from 'react';
 
 import { gettext } from '../../utils/gettext';
-import { ADMIN_URLS, LOCALE_NAMES } from '../../config/wagtailConfig';
+import { LOCALE_NAMES, WAGTAIL_CONFIG } from '../../config/wagtailConfig';
 import Icon from '../Icon/Icon';
 import Link from '../Link/Link';
 import PublicationStatus from '../PublicationStatus/PublicationStatus';
 import { PageState } from './reducers/nodes';
 
+const { ADMIN_URLS } = WAGTAIL_CONFIG;
+
 // Hoist icons in the explorer item, as it is re-rendered many times.
 const childrenIcon = <Icon name="folder-inverse" className="icon--menuitem" />;
 

+ 11 - 12
client/src/config/wagtailConfig.test.js

@@ -1,32 +1,31 @@
 import {
-  ADMIN_API,
-  ADMIN_URLS,
+  LOCALE_NAMES,
   MAX_EXPLORER_PAGES,
   WAGTAIL_CONFIG,
 } from './wagtailConfig';
 
 describe('wagtailConfig', () => {
-  describe('ADMIN_API', () => {
+  describe('LOCALE_NAMES', () => {
     it('exists', () => {
-      expect(ADMIN_API).toBeDefined();
-    });
-  });
-
-  describe('ADMIN_URLS', () => {
-    it('exists', () => {
-      expect(ADMIN_URLS).toBeDefined();
+      expect(LOCALE_NAMES).toBeInstanceOf(Map);
+      expect(LOCALE_NAMES.get('fr')).toEqual('French');
     });
   });
 
   describe('MAX_EXPLORER_PAGES', () => {
     it('exists', () => {
-      expect(MAX_EXPLORER_PAGES).toBeDefined();
+      expect(MAX_EXPLORER_PAGES).toBeGreaterThan(0);
     });
   });
 
   describe('WAGTAIL_CONFIG', () => {
     it('exists', () => {
-      expect(WAGTAIL_CONFIG).toBeDefined();
+      expect(WAGTAIL_CONFIG).toEqual(
+        expect.objectContaining({
+          ACTIVE_LOCALE: expect.any(String),
+          LOCALES: expect.any(Array),
+        }),
+      );
     });
   });
 });

+ 25 - 19
client/src/config/wagtailConfig.ts

@@ -1,22 +1,13 @@
-export const { ADMIN_API } = global.wagtailConfig;
-export const { ADMIN_URLS } = global.wagtailConfig;
-
-// Maximum number of pages to load inside the explorer menu.
-export const MAX_EXPLORER_PAGES = 200;
-
-export const LOCALE_NAMES = new Map();
-
-/* eslint-disable-next-line camelcase */
-global.wagtailConfig.LOCALES.forEach(({ code, display_name }) => {
-  LOCALE_NAMES.set(code, display_name);
-});
-
-function getWagtailConfig() {
-  // TODO: Move window.wagtailConfig from the base HTML template
-  // to the wagtail-config JSON script.
+import type { WagtailConfig } from '../custom.d';
 
+const getWagtailConfig = (
+  config = (global as any).wagtailConfig as WagtailConfig,
+) => {
+  // Avoid re-parsing the JSON if global has been already created in core.js
+  if (config) return config;
   try {
-    return JSON.parse(document.getElementById('wagtail-config')?.textContent);
+    const json = document.getElementById('wagtail-config')?.textContent || '';
+    return JSON.parse(json);
   } catch (err) {
     /* eslint-disable no-console */
     console.error('Error loading Wagtail config');
@@ -28,6 +19,21 @@ function getWagtailConfig() {
     // valid JSON, ignore it and return an empty object.
     return {};
   }
-}
+};
+
+const config = getWagtailConfig() as WagtailConfig;
+
+/**
+ * Maximum number of pages to load inside the explorer menu.
+ */
+export const MAX_EXPLORER_PAGES = 200;
+
+export const LOCALE_NAMES = (config.LOCALES || []).reduce(
+  (locales, { code, display_name: displayName }) => {
+    locales.set(code, displayName);
+    return locales;
+  },
+  new Map<string, string>(),
+);
 
-export const WAGTAIL_CONFIG = getWagtailConfig();
+export { config as WAGTAIL_CONFIG };

+ 19 - 24
client/src/custom.d.ts

@@ -1,4 +1,22 @@
-export {};
+export interface WagtailConfig {
+  ADMIN_API: {
+    PAGES: string;
+    DOCUMENTS: string;
+    IMAGES: string;
+    EXTRA_CHILDREN_PARAMETERS: string;
+  };
+  ADMIN_URLS: {
+    DISMISSIBLES: string;
+    PAGES: string;
+  };
+  CSRF_HEADER_NAME: string;
+  CSRF_TOKEN: string;
+  I18N_ENABLED: boolean;
+  LOCALES: {
+    code: string;
+    display_name: string;
+  }[];
+}
 
 declare global {
   interface Window {
@@ -6,28 +24,5 @@ declare global {
     telepath: any;
   }
 
-  // Get text
-
-  // Wagtail globals
-
-  interface WagtailConfig {
-    ADMIN_API: {
-      PAGES: string;
-      DOCUMENTS: string;
-      IMAGES: string;
-      EXTRA_CHILDREN_PARAMETERS: string;
-    };
-
-    ADMIN_URLS: {
-      PAGES: string;
-    };
-
-    I18N_ENABLED: boolean;
-    LOCALES: {
-      code: string;
-
-      display_name: string;
-    }[];
-  }
   const wagtailConfig: WagtailConfig;
 }

+ 4 - 0
client/src/entrypoints/admin/core.js

@@ -5,6 +5,7 @@ import { Icon, Portal } from '../..';
 import { coreControllerDefinitions } from '../../controllers';
 import { InlinePanel } from '../../components/InlinePanel';
 import { MultipleChooserPanel } from '../../components/MultipleChooserPanel';
+import { WAGTAIL_CONFIG } from '../../config/wagtailConfig';
 import { initStimulus } from '../../includes/initStimulus';
 
 import { urlify } from '../../utils/urlify';
@@ -32,6 +33,9 @@ wagtail.app = initStimulus({ definitions: coreControllerDefinitions });
 /** Expose components as globals for third-party reuse. */
 wagtail.components = { Icon, Portal };
 
+/** Expose a global for undocumented third-party usage. */
+window.wagtailConfig = WAGTAIL_CONFIG;
+
 window.wagtail = wagtail;
 
 window.escapeHtml = escapeHtml;

+ 8 - 9
client/tests/stubs.js

@@ -8,7 +8,7 @@
  * See /wagtailadmin/templates/wagtailadmin/admin_base.html.
  */
 
-global.wagtailConfig = {
+const wagtailConfig = {
   ADMIN_API: {
     DOCUMENTS: '/admin/api/main/documents/',
     IMAGES: '/admin/api/main/images/',
@@ -18,6 +18,8 @@ global.wagtailConfig = {
   ADMIN_URLS: {
     PAGES: '/admin/pages/',
   },
+  CSRF_HEADER_NAME: 'x-xsrf-token',
+  CSRF_TOKEN: 'potato',
   DATE_FORMATTING: {
     DATE_FORMAT: 'MMM. D, YYYY',
     SHORT_DATE_FORMAT: 'DD/MM/YYYY',
@@ -36,16 +38,13 @@ global.wagtailConfig = {
   ACTIVE_LOCALE: 'en',
 };
 
-const script = document.createElement('script');
-script.type = 'application/json';
-script.id = 'wagtail-config';
-script.textContent = JSON.stringify({
-  CSRF_HEADER_NAME: 'x-xsrf-token',
-  CSRF_TOKEN: 'potato',
+const configScript = Object.assign(document.createElement('script'), {
+  id: 'wagtail-config',
+  textContent: JSON.stringify(wagtailConfig),
+  type: 'application/json',
 });
-document.body.appendChild(script);
 
-global.wagtailVersion = '1.6a1';
+document.body.appendChild(configScript);
 
 global.wagtail = {};
 

+ 0 - 28
wagtail/admin/templates/wagtailadmin/admin_base.html

@@ -14,34 +14,6 @@
 {% endblock %}
 
 {% block js %}
-    <script>
-        (function(document, window) {
-            window.wagtailConfig = window.wagtailConfig || {};
-            wagtailConfig.ADMIN_API = {
-                PAGES: '{% url "wagtailadmin_api:pages:listing" %}',
-                DOCUMENTS: '{% url "wagtailadmin_api:documents:listing" %}',
-                IMAGES: '{% url "wagtailadmin_api:images:listing" %}',
-                {# // Use this to add an extra query string on all API requests. #}
-                {# // Example value: '&order=-id' #}
-                EXTRA_CHILDREN_PARAMETERS: '',
-            };
-
-            {% i18n_enabled as i18n_enabled %}
-            {% locales as locales %}
-            wagtailConfig.I18N_ENABLED = {% if i18n_enabled %}true{% else %}false{% endif %};
-            wagtailConfig.LOCALES = {{ locales|safe }};
-
-            {% if locale %}
-                wagtailConfig.ACTIVE_CONTENT_LOCALE = '{{ locale.language_code }}'
-            {% endif %}
-
-            wagtailConfig.STRINGS = {% js_translation_strings %};
-
-            wagtailConfig.ADMIN_URLS = {
-                PAGES: '{% url "wagtailadmin_explore_root" %}'
-            };
-        })(document, window);
-    </script>
     {% wagtail_config as config %}
     {{ config|json_script:"wagtail-config" }}
     <script src="{% versioned_static 'wagtailadmin/js/vendor/jquery-3.6.0.min.js' %}"></script>

+ 24 - 10
wagtail/admin/templatetags/wagtailadmin_tags.py

@@ -859,16 +859,15 @@ def i18n_enabled():
 
 
 @register.simple_tag
-def locales():
-    return json.dumps(
-        [
-            {
-                "code": locale.language_code,
-                "display_name": force_str(locale.get_display_name()),
-            }
-            for locale in Locale.objects.all()
-        ]
-    )
+def locales(serialize=True):
+    result = [
+        {
+            "code": locale.language_code,
+            "display_name": force_str(locale.get_display_name()),
+        }
+        for locale in Locale.objects.all()
+    ]
+    return json.dumps(result) if serialize else result
 
 
 @register.simple_tag
@@ -936,10 +935,25 @@ def wagtail_config(context):
         "CSRF_HEADER_NAME": HttpHeaders.parse_header_name(
             getattr(settings, "CSRF_HEADER_NAME")
         ),
+        "ADMIN_API": {
+            "PAGES": reverse("wagtailadmin_api:pages:listing"),
+            "DOCUMENTS": reverse("wagtailadmin_api:documents:listing"),
+            "IMAGES": reverse("wagtailadmin_api:images:listing"),
+            # Used to add an extra query string on all API requests. Example value: '&order=-id'
+            "EXTRA_CHILDREN_PARAMETERS": "",
+        },
         "ADMIN_URLS": {
             "DISMISSIBLES": reverse("wagtailadmin_dismissibles"),
+            "PAGES": reverse("wagtailadmin_explore_root"),
         },
+        "I18N_ENABLED": i18n_enabled(),
+        "LOCALES": locales(serialize=False),
+        "STRINGS": get_js_translation_strings(),
     }
+
+    if locale := context.get("locale"):
+        config["ACTIVE_CONTENT_LOCALE"] = locale.language_code
+
     return config