From 1da3e5fde729e7ae17c62a21f5d90c3b8b047d9d Mon Sep 17 00:00:00 2001 From: LB Johnston Date: Fri, 2 Jun 2023 19:24:46 +1000 Subject: [PATCH] Refactor Stimulus util & module export - Accessing constructor was confusing and createController is not a core requirement - Avoid modifying the base Stimulus application class - Expose `window.StimulusModule` as the module output - Expose `window.wagtail.app` as the Wagtail Stimulus application instance - Rename root element variable to `root` in initStimulus (so we do not conflict with potential action option registration that uses `element` variable names) - Pull in the wagtail.components to the same core.js file and add JSDoc for exposed global - Relates to #10197 --- client/src/entrypoints/admin/core.js | 35 ++++-- client/src/entrypoints/admin/core.test.js | 32 ++++++ client/src/entrypoints/admin/wagtailadmin.js | 7 -- .../entrypoints/admin/wagtailadmin.test.js | 17 --- client/src/includes/initStimulus.test.js | 105 +----------------- client/src/includes/initStimulus.ts | 77 ++----------- client/storybook/StimulusWrapper.tsx | 4 +- 7 files changed, 71 insertions(+), 206 deletions(-) create mode 100644 client/src/entrypoints/admin/core.test.js delete mode 100644 client/src/entrypoints/admin/wagtailadmin.test.js diff --git a/client/src/entrypoints/admin/core.js b/client/src/entrypoints/admin/core.js index b7fe71ca92d0..aeccbbac22c9 100644 --- a/client/src/entrypoints/admin/core.js +++ b/client/src/entrypoints/admin/core.js @@ -1,11 +1,34 @@ import $ from 'jquery'; +import * as StimulusModule from '@hotwired/stimulus'; +import { Icon, Portal } from '../..'; import { coreControllerDefinitions } from '../../controllers'; import { escapeHtml } from '../../utils/text'; import { initStimulus } from '../../includes/initStimulus'; -/** initialise Wagtail Stimulus application with core controller definitions */ -window.Stimulus = initStimulus({ definitions: coreControllerDefinitions }); +/** Expose a global to allow for customisations and packages to build with Stimulus. */ +window.StimulusModule = StimulusModule; + +/** + * Wagtail global module, useful for debugging and as the exposed + * interface to access the Stimulus application instance and base + * React components. + * + * @type {Object} wagtail + * @property {Object} app - Wagtail's Stimulus application instance. + * @property {Object} components - Exposed components as globals for third-party reuse. + * @property {Object} components.Icon - Icon React component. + * @property {Object} components.Portal - Portal React component. + */ +const wagtail = window.wagtail || {}; + +/** Initialise Wagtail Stimulus application with core controller definitions. */ +wagtail.app = initStimulus({ definitions: coreControllerDefinitions }); + +/** Expose components as globals for third-party reuse. */ +wagtail.components = { Icon, Portal }; + +window.wagtail = wagtail; window.escapeHtml = escapeHtml; @@ -225,11 +248,3 @@ $(() => { $(this).removeClass('hovered'); }); }); - -// ============================================================================= -// Wagtail global module, mainly useful for debugging. -// ============================================================================= - -const wagtail = window.wagtail || {}; - -window.wagtail = wagtail; diff --git a/client/src/entrypoints/admin/core.test.js b/client/src/entrypoints/admin/core.test.js new file mode 100644 index 000000000000..98e9947b92a1 --- /dev/null +++ b/client/src/entrypoints/admin/core.test.js @@ -0,0 +1,32 @@ +jest.mock('../..'); + +document.addEventListener = jest.fn(); + +require('./core'); + +describe('core', () => { + const [event] = document.addEventListener.mock.calls[0]; + + it('exposes the Stimulus application instance for reuse', () => { + expect(Object.keys(window.wagtail.app)).toEqual( + expect.arrayContaining(['debug', 'logger']), + ); + + expect(window.wagtail.app.load).toBeInstanceOf(Function); + expect(window.wagtail.app.register).toBeInstanceOf(Function); + }); + + it('exposes components for reuse', () => { + expect(Object.keys(window.wagtail.components)).toEqual(['Icon', 'Portal']); + }); + + it('exposes the Stimulus module for reuse', () => { + expect(Object.keys(window.StimulusModule)).toEqual( + expect.arrayContaining(['Application', 'Controller']), + ); + }); + + it('DOMContentLoaded', () => { + expect(event).toBe('DOMContentLoaded'); + }); +}); diff --git a/client/src/entrypoints/admin/wagtailadmin.js b/client/src/entrypoints/admin/wagtailadmin.js index 066237423984..c223d623539b 100644 --- a/client/src/entrypoints/admin/wagtailadmin.js +++ b/client/src/entrypoints/admin/wagtailadmin.js @@ -1,4 +1,3 @@ -import { Icon, Portal } from '../..'; import { initTooltips } from '../../includes/initTooltips'; import { initTabs } from '../../includes/tabs'; import initSidePanel from '../../includes/sidePanel'; @@ -8,12 +7,6 @@ import { } from '../../includes/panels'; import { initMinimap } from '../../components/Minimap'; -// Expose components as globals for third-party reuse. -window.wagtail.components = { - Icon, - Portal, -}; - /** * Add in here code to run once the page is loaded. */ diff --git a/client/src/entrypoints/admin/wagtailadmin.test.js b/client/src/entrypoints/admin/wagtailadmin.test.js deleted file mode 100644 index 4d2da2fbb83c..000000000000 --- a/client/src/entrypoints/admin/wagtailadmin.test.js +++ /dev/null @@ -1,17 +0,0 @@ -jest.mock('../..'); - -document.addEventListener = jest.fn(); - -require('./wagtailadmin'); - -describe('wagtailadmin', () => { - const [event] = document.addEventListener.mock.calls[0]; - - it('exposes components for reuse', () => { - expect(Object.keys(window.wagtail.components)).toEqual(['Icon', 'Portal']); - }); - - it('DOMContentLoaded', () => { - expect(event).toBe('DOMContentLoaded'); - }); -}); diff --git a/client/src/includes/initStimulus.test.js b/client/src/includes/initStimulus.test.js index 67735ca9d347..f005a276f096 100644 --- a/client/src/includes/initStimulus.test.js +++ b/client/src/includes/initStimulus.test.js @@ -4,36 +4,7 @@ import { initStimulus } from './initStimulus'; jest.useFakeTimers(); /** - * Example controller (shortcut method definitions object) from documentation - */ -const wordCountController = { - STATIC: { - values: { max: { default: 10, type: Number } }, - }, - connect() { - this.setupOutput(); - this.updateCount(); - }, - setupOutput() { - if (this.output) return; - const template = document.createElement('template'); - template.innerHTML = ``; - const output = template.content.firstChild; - this.element.insertAdjacentElement('beforebegin', output); - this.output = output; - }, - updateCount(event) { - const value = event ? event.target.value : this.element.value; - const words = (value || '').split(' '); - this.output.textContent = `${words.length} / ${this.maxValue} words`; - }, - disconnect() { - this.output && this.output.remove(); - }, -}; - -/** - * Example controller from documentation as an ES6 class + * Example controller. */ class WordCountController extends Controller { static values = { max: { default: 10, type: Number } }; @@ -51,7 +22,7 @@ class WordCountController extends Controller { setupOutput() { if (this.output) return; const template = document.createElement('template'); - template.innerHTML = ``; + template.innerHTML = ``; const output = template.content.firstChild; this.element.insertAdjacentElement('beforebegin', output); this.output = output; @@ -115,52 +86,6 @@ describe('initStimulus', () => { expect(application.controllers[0]).toBeInstanceOf(TestMockController); }); - it('should support registering a controller via an object with the createController static method', async () => { - const section = document.createElement('section'); - section.id = 'example-a'; - section.innerHTML = ``; - - // create a controller and register it - application.register( - 'example-a', - application.constructor.createController(wordCountController), - ); - - // before controller element added - should not include an `output` element - expect(document.querySelector('#example-a > output')).toEqual(null); - - document.querySelector('section').after(section); - - await Promise.resolve(); - - // after controller connected - should have an output element - expect(document.querySelector('#example-a > output').innerHTML).toEqual( - '2 / 10 words', - ); - - await Promise.resolve(); - - // should respond to changes on the input - const input = document.querySelector('#example-a > input'); - input.setAttribute('value', 'even more words'); - input.dispatchEvent(new Event('change')); - - expect(document.querySelector('#example-a > output').innerHTML).toEqual( - '3 / 10 words', - ); - - // removal of the input should also remove the output (disconnect method) - input.remove(); - - await Promise.resolve(); - - // should call the disconnect method (removal of the injected HTML) - expect(document.querySelector('#example-a > output')).toEqual(null); - - // clean up - section.remove(); - }); - it('should support the documented approach for registering a controller via a class with register', async () => { const section = document.createElement('section'); section.id = 'example-b'; @@ -203,30 +128,4 @@ describe('initStimulus', () => { // clean up section.remove(); }); - - it('should provide access to a base Controller class on the returned application instance', () => { - expect(application.constructor.Controller).toEqual(Controller); - }); -}); - -describe('createController', () => { - const createController = initStimulus().constructor.createController; - - it('should safely create a Stimulus Controller class if no args provided', () => { - const CustomController = createController(); - expect(CustomController.prototype instanceof Controller).toBeTruthy(); - }); - - it('should create a Stimulus Controller class with static properties', () => { - const someMethod = jest.fn(); - - const CustomController = createController({ - STATIC: { targets: ['source'] }, - someMethod, - }); - - expect(CustomController.targets).toEqual(['source']); - expect(CustomController.someMethod).toBeUndefined(); - expect(CustomController.prototype.someMethod).toEqual(someMethod); - }); }); diff --git a/client/src/includes/initStimulus.ts b/client/src/includes/initStimulus.ts index c00fad6515d1..07fad91428b2 100644 --- a/client/src/includes/initStimulus.ts +++ b/client/src/includes/initStimulus.ts @@ -1,81 +1,24 @@ import type { Definition } from '@hotwired/stimulus'; -import { Application, Controller } from '@hotwired/stimulus'; - -type ControllerObjectDefinition = Record void> & { - STATIC?: { - classes?: string[]; - targets?: string[]; - values: typeof Controller.values; - }; -}; - -/** - * Extend the Stimulus application class to provide some convenience - * static attributes or methods to be accessed globally. - */ -class WagtailApplication extends Application { - /** - * Ensure the base Controller class is available for new controllers. - */ - static Controller = Controller; - - /** - * Function that accepts a plain old object and returns a Stimulus Controller. - * Useful when ES6 modules with base class being extended not in use - * or build tool not in use or for just super convenient class creation. - * - * Inspired heavily by - * https://github.com/StackExchange/Stacks/blob/v1.6.5/lib/ts/stacks.ts#L84 - * - * @example - * createController({ - * STATIC: { targets = ['container'] } - * connect() { - * console.log('connected', this.element, this.containerTarget); - * } - * }) - * - */ - static createController = ( - controllerDefinition: ControllerObjectDefinition = {}, - ): typeof Controller => { - class NewController extends Controller {} - - const { STATIC = {}, ...controllerDefinitionWithoutStatic } = - controllerDefinition; - - // set up static values - Object.entries(STATIC).forEach(([key, value]) => { - NewController[key] = value; - }); - - // set up class methods - Object.assign(NewController.prototype, controllerDefinitionWithoutStatic); - - return NewController; - }; -} +import { Application } from '@hotwired/stimulus'; /** - * Initialises the Wagtail Stimulus application and dispatches and registers - * custom event behaviour. + * Initialises the Wagtail Stimulus application, loads the provided controller + * definitions and returns the app instance. * * Loads the supplied core controller definitions into the application. - * Turns on debug mode if in local development (for now). + * Turns on debug mode if in local development. */ export const initStimulus = ({ debug = process.env.NODE_ENV === 'development', definitions = [], - element = document.documentElement, + root = document.documentElement, }: { debug?: boolean; definitions?: Definition[]; - element?: HTMLElement; + root?: HTMLElement; } = {}): Application => { - const application = WagtailApplication.start(element); - - application.debug = debug; - application.load(definitions); - - return application; + const app = Application.start(root); + app.debug = debug; + app.load(definitions); + return app; }; diff --git a/client/storybook/StimulusWrapper.tsx b/client/storybook/StimulusWrapper.tsx index 5150b61bfa92..e06f85675a45 100644 --- a/client/storybook/StimulusWrapper.tsx +++ b/client/storybook/StimulusWrapper.tsx @@ -32,8 +32,8 @@ export class StimulusWrapper extends React.Component<{ componentDidMount() { const { debug = false, definitions = [] } = this.props; - const element = this.ref.current || document.documentElement; - this.application = initStimulus({ debug, definitions, element }); + const root = this.ref.current || document.documentElement; + this.application = initStimulus({ debug, definitions, root }); } componentDidUpdate({ debug: prevDebug }) {