From 13c66323e63fe833528e28cb5d42c2a48b626dad Mon Sep 17 00:00:00 2001 From: Ryan Weaver Date: Wed, 19 Oct 2022 11:03:51 -0400 Subject: [PATCH] Adding a data-model system for sync'ing data between parent and child components This also includes the dispatching of several new PHP events. --- package.json | 5 +- .../Resources/assets/test/controller.test.ts | 2 +- src/LiveComponent/CHANGELOG.md | 2 +- .../assets/src/Component/index.ts | 78 ++++++++-- .../src/Component/plugins/LoadingPlugin.ts | 2 +- .../src/Component/plugins/PollingPlugin.ts | 2 +- .../src/{ => Directive}/directives_parser.ts | 0 .../assets/src/Directive/get_model_binding.ts | 52 +++++++ src/LiveComponent/assets/src/HookManager.ts | 12 ++ src/LiveComponent/assets/src/dom_utils.ts | 27 +++- .../assets/src/live_controller.ts | 70 ++++----- .../{ => Directive}/directives_parser.test.ts | 2 +- .../test/Directive/get_model_binding.test.ts | 37 +++++ .../test/controller/child-model.test.ts | 140 ++++++++++++++++++ .../assets/test/controller/child.test.ts | 26 +--- .../assets/test/controller/model.test.ts | 2 +- .../assets/test/dom_utils.test.ts | 6 +- src/LiveComponent/assets/test/tools.ts | 30 +++- src/LiveComponent/src/Resources/doc/index.rst | 73 +++++---- .../assets/src/turbo_stream_controller.ts | 4 +- src/TwigComponent/CHANGELOG.md | 8 +- src/TwigComponent/src/ComponentFactory.php | 12 ++ .../Compiler/TwigComponentPass.php | 2 +- .../TwigComponentExtension.php | 10 ++ .../src/Event/PostMountEvent.php | 41 +++++ src/TwigComponent/src/Event/PreMountEvent.php | 41 +++++ .../DataModelPropsSubscriber.php | 77 ++++++++++ src/TwigComponent/src/Resources/doc/index.rst | 15 +- .../src/Util/ModelBindingParser.php | 56 +++++++ .../Component/ParentFormComponent.php | 23 +++ .../Fixtures/Component/TextareaComponent.php | 21 +++ .../parent_form_component.html.twig | 11 ++ .../components/textarea_component.html.twig | 1 + .../DataModelPropsSubscriberTest.php | 33 +++++ .../Unit/Util/ModelBindingParserTest.php | 54 +++++++ .../assets/test/render_controller.test.ts | 2 +- 36 files changed, 853 insertions(+), 126 deletions(-) rename src/LiveComponent/assets/src/{ => Directive}/directives_parser.ts (100%) create mode 100644 src/LiveComponent/assets/src/Directive/get_model_binding.ts rename src/LiveComponent/assets/test/{ => Directive}/directives_parser.test.ts (98%) create mode 100644 src/LiveComponent/assets/test/Directive/get_model_binding.test.ts create mode 100644 src/LiveComponent/assets/test/controller/child-model.test.ts create mode 100644 src/TwigComponent/src/Event/PostMountEvent.php create mode 100644 src/TwigComponent/src/Event/PreMountEvent.php create mode 100644 src/TwigComponent/src/EventListener/DataModelPropsSubscriber.php create mode 100644 src/TwigComponent/src/Util/ModelBindingParser.php create mode 100644 src/TwigComponent/tests/Fixtures/Component/ParentFormComponent.php create mode 100644 src/TwigComponent/tests/Fixtures/Component/TextareaComponent.php create mode 100644 src/TwigComponent/tests/Fixtures/templates/components/parent_form_component.html.twig create mode 100644 src/TwigComponent/tests/Fixtures/templates/components/textarea_component.html.twig create mode 100644 src/TwigComponent/tests/Integration/EventListener/DataModelPropsSubscriberTest.php create mode 100644 src/TwigComponent/tests/Unit/Util/ModelBindingParserTest.php diff --git a/package.json b/package.json index 12524da414d..e16d02d010f 100644 --- a/package.json +++ b/package.json @@ -50,7 +50,10 @@ "@typescript-eslint/no-explicit-any": "off", "@typescript-eslint/no-empty-function": "off", "@typescript-eslint/ban-ts-comment": "off", - "quotes": ["error", "single"] + "quotes": [ + "error", + "single" + ] }, "env": { "browser": true diff --git a/src/Cropperjs/Resources/assets/test/controller.test.ts b/src/Cropperjs/Resources/assets/test/controller.test.ts index 6027c9e0f9f..4b5d6b11dff 100644 --- a/src/Cropperjs/Resources/assets/test/controller.test.ts +++ b/src/Cropperjs/Resources/assets/test/controller.test.ts @@ -52,7 +52,7 @@ describe('CropperjsController', () => { data-cropperjs-public-url-value="https://symfony.com/logos/symfony_black_02.png" data-cropperjs-options-value="${dataToJsonAttribute({ viewMode: 1, - dragMode: "move" + dragMode: 'move' })}" > diff --git a/src/LiveComponent/CHANGELOG.md b/src/LiveComponent/CHANGELOG.md index fe5ac6fdab1..421e0ac1408 100644 --- a/src/LiveComponent/CHANGELOG.md +++ b/src/LiveComponent/CHANGELOG.md @@ -28,7 +28,7 @@ - [BEHAVIOR CHANGE] The way that child components re-render when a parent re-renders has changed, but shouldn't be drastically different. Child components will now - avoid re-rendering if no "input" to the component changed *and* will maintain + avoid re-rendering if no "input" to the component changed _and_ will maintain any writable `LiveProp` values after the re-render. Also, the re-render happens in a separate Ajax call after the parent has finished re-rendering. diff --git a/src/LiveComponent/assets/src/Component/index.ts b/src/LiveComponent/assets/src/Component/index.ts index 85c1d5c06b8..795435fa412 100644 --- a/src/LiveComponent/assets/src/Component/index.ts +++ b/src/LiveComponent/assets/src/Component/index.ts @@ -11,9 +11,20 @@ import { ElementDriver } from './ElementDriver'; import HookManager from '../HookManager'; import { PluginInterface } from './plugins/PluginInterface'; import BackendResponse from '../BackendResponse'; +import { ModelBinding } from '../Directive/get_model_binding'; declare const Turbo: any; +class ChildComponentWrapper { + component: Component; + modelBindings: ModelBinding[]; + + constructor(component: Component, modelBindings: ModelBinding[]) { + this.component = component; + this.modelBindings = modelBindings; + } +} + export default class Component { readonly element: HTMLElement; private readonly backend: BackendInterface; @@ -46,7 +57,7 @@ export default class Component { private nextRequestPromise: Promise; private nextRequestPromiseResolve: (response: BackendResponse) => any; - private children: Map = new Map(); + private children: Map = new Map(); private parent: Component|null = null; /** @@ -69,6 +80,8 @@ export default class Component { this.unsyncedInputsTracker = new UnsyncedInputsTracker(this, elementDriver); this.hooks = new HookManager(); this.resetPromise(); + + this.onChildComponentModelUpdate = this.onChildComponentModelUpdate.bind(this); } addPlugin(plugin: PluginInterface) { @@ -95,18 +108,22 @@ export default class Component { * * render:finished (component: Component) => {} * * loading.state:started (element: HTMLElement, request: BackendRequest) => {} * * loading.state:finished (element: HTMLElement) => {} - * * model:set (model: string, value: any) => {} + * * model:set (model: string, value: any, component: Component) => {} */ on(hookName: string, callback: (...args: any[]) => void): void { this.hooks.register(hookName, callback); } + off(hookName: string, callback: (...args: any[]) => void): void { + this.hooks.unregister(hookName, callback); + } + set(model: string, value: any, reRender = false, debounce: number|boolean = false): Promise { const promise = this.nextRequestPromise; const modelName = normalizeModelName(model); const isChanged = this.valueStore.set(modelName, value); - this.hooks.triggerHook('model:set', model, value); + this.hooks.triggerHook('model:set', model, value, this); // the model's data is no longer unsynced this.unsyncedInputsTracker.markModelAsSynced(modelName); @@ -151,13 +168,14 @@ export default class Component { return this.unsyncedInputsTracker.getModifiedModels(); } - addChild(component: Component): void { - if (!component.id) { + addChild(child: Component, modelBindings: ModelBinding[] = []): void { + if (!child.id) { throw new Error('Children components must have an id.'); } - this.children.set(component.id, component); - component.parent = this; + this.children.set(child.id, new ChildComponentWrapper(child, modelBindings)); + child.parent = this; + child.on('model:set', this.onChildComponentModelUpdate); } removeChild(child: Component): void { @@ -167,6 +185,7 @@ export default class Component { this.children.delete(child.id); child.parent = null; + child.off('model:set', this.onChildComponentModelUpdate); } getParent(): Component|null { @@ -174,9 +193,19 @@ export default class Component { } getChildren(): Map { - return new Map(this.children); + const children: Map = new Map(); + this.children.forEach((childComponent, id) => { + children.set(id, childComponent.component); + }); + + return children; } + /** + * Called during morphdom: read props from toEl and re-render if necessary. + * + * @param toEl + */ updateFromNewElement(toEl: HTMLElement): boolean { const props = this.elementDriver.getComponentProps(toEl); @@ -201,6 +230,36 @@ export default class Component { return false; } + /** + * Handles data-model binding from a parent component onto a child. + */ + onChildComponentModelUpdate(modelName: string, value: any, childComponent: Component): void { + if (!childComponent.id) { + throw new Error('Missing id'); + } + + const childWrapper = this.children.get(childComponent.id); + if (!childWrapper) { + throw new Error('Missing child'); + } + + childWrapper.modelBindings.forEach((modelBinding) => { + const childModelName = modelBinding.innerModelName || 'value'; + + // skip, unless childModelName matches the model that just changed + if (childModelName !== modelName) { + return; + } + + this.set( + modelBinding.modelName, + value, + modelBinding.shouldRender, + modelBinding.debounce + ); + }); + } + private tryStartingRequest(): void { if (!this.backendRequest) { this.performRequest() @@ -391,7 +450,8 @@ export default class Component { private getChildrenFingerprints(): any { const fingerprints: any = {}; - this.children.forEach((child) => { + this.children.forEach((childComponent) => { + const child = childComponent.component; if (!child.id) { throw new Error('missing id'); } diff --git a/src/LiveComponent/assets/src/Component/plugins/LoadingPlugin.ts b/src/LiveComponent/assets/src/Component/plugins/LoadingPlugin.ts index 5d60722675f..6dcad90789b 100644 --- a/src/LiveComponent/assets/src/Component/plugins/LoadingPlugin.ts +++ b/src/LiveComponent/assets/src/Component/plugins/LoadingPlugin.ts @@ -2,7 +2,7 @@ import { Directive, DirectiveModifier, parseDirectives -} from '../../directives_parser'; +} from '../../Directive/directives_parser'; import { combineSpacedArray} from '../../string_utils'; import BackendRequest from '../../BackendRequest'; import Component from '../../Component'; diff --git a/src/LiveComponent/assets/src/Component/plugins/PollingPlugin.ts b/src/LiveComponent/assets/src/Component/plugins/PollingPlugin.ts index 11a3ed0d5b8..a74c4745415 100644 --- a/src/LiveComponent/assets/src/Component/plugins/PollingPlugin.ts +++ b/src/LiveComponent/assets/src/Component/plugins/PollingPlugin.ts @@ -1,5 +1,5 @@ import Component from '../index'; -import { parseDirectives } from '../../directives_parser'; +import { parseDirectives } from '../../Directive/directives_parser'; import PollingDirector from '../../PollingDirector'; import { PluginInterface } from './PluginInterface'; diff --git a/src/LiveComponent/assets/src/directives_parser.ts b/src/LiveComponent/assets/src/Directive/directives_parser.ts similarity index 100% rename from src/LiveComponent/assets/src/directives_parser.ts rename to src/LiveComponent/assets/src/Directive/directives_parser.ts diff --git a/src/LiveComponent/assets/src/Directive/get_model_binding.ts b/src/LiveComponent/assets/src/Directive/get_model_binding.ts new file mode 100644 index 00000000000..587f38e05e9 --- /dev/null +++ b/src/LiveComponent/assets/src/Directive/get_model_binding.ts @@ -0,0 +1,52 @@ +import {Directive} from './directives_parser'; + +export interface ModelBinding { + modelName: string, + innerModelName: string|null, + shouldRender: boolean, + debounce: number|boolean, + targetEventName: string|null +} + +export default function(modelDirective: Directive): ModelBinding { + let shouldRender = true; + let targetEventName = null; + let debounce: number|boolean = false; + + modelDirective.modifiers.forEach((modifier) => { + switch (modifier.name) { + case 'on': + if (!modifier.value) { + throw new Error(`The "on" modifier in ${modelDirective.getString()} requires a value - e.g. on(change).`); + } + if (!['input', 'change'].includes(modifier.value)) { + throw new Error(`The "on" modifier in ${modelDirective.getString()} only accepts the arguments "input" or "change".`); + } + + targetEventName = modifier.value; + + break; + case 'norender': + shouldRender = false; + + break; + + case 'debounce': + debounce = modifier.value ? parseInt(modifier.value) : true; + + break; + default: + throw new Error(`Unknown modifier "${modifier.name}" in data-model="${modelDirective.getString()}".`); + } + }); + + const [ modelName, innerModelName ] = modelDirective.action.split(':'); + + return { + modelName, + innerModelName: innerModelName || null, + shouldRender, + debounce, + targetEventName + } +} diff --git a/src/LiveComponent/assets/src/HookManager.ts b/src/LiveComponent/assets/src/HookManager.ts index ac78b7af6b7..755343ca8b3 100644 --- a/src/LiveComponent/assets/src/HookManager.ts +++ b/src/LiveComponent/assets/src/HookManager.ts @@ -16,6 +16,18 @@ export default class { this.hooks.set(hookName, hooks); } + unregister(hookName: string, callback: () => void): void { + const hooks = this.hooks.get(hookName) || []; + + const index = hooks.indexOf(callback); + if (index === -1) { + return; + } + + hooks.splice(index, 1); + this.hooks.set(hookName, hooks); + } + triggerHook(hookName: string, ...args: any[]): void { const hooks = this.hooks.get(hookName) || []; hooks.forEach((callback) => { diff --git a/src/LiveComponent/assets/src/dom_utils.ts b/src/LiveComponent/assets/src/dom_utils.ts index 836b475bce8..76cf16daf9b 100644 --- a/src/LiveComponent/assets/src/dom_utils.ts +++ b/src/LiveComponent/assets/src/dom_utils.ts @@ -1,5 +1,5 @@ import ValueStore from './Component/ValueStore'; -import { Directive, parseDirectives } from './directives_parser'; +import { Directive, parseDirectives } from './Directive/directives_parser'; import { normalizeModelName } from './string_utils'; import Component from './Component'; @@ -111,18 +111,33 @@ export function setValueOnElement(element: HTMLElement, value: any): void { (element as HTMLInputElement).value = value } -export function getModelDirectiveFromElement(element: HTMLElement, throwOnMissing = true): null|Directive { - if (element.dataset.model) { - const directives = parseDirectives(element.dataset.model); - const directive = directives[0]; +/** + * Fetches *all* "data-model" directives for a given element. + * + * @param element + */ +export function getAllModelDirectiveFromElements(element: HTMLElement): Directive[] { + if (!element.dataset.model) { + return []; + } + const directives = parseDirectives(element.dataset.model); + + directives.forEach((directive) => { if (directive.args.length > 0 || directive.named.length > 0) { throw new Error(`The data-model="${element.dataset.model}" format is invalid: it does not support passing arguments to the model.`); } directive.action = normalizeModelName(directive.action); + }); - return directive; + return directives; +} + +export function getModelDirectiveFromElement(element: HTMLElement, throwOnMissing = true): null|Directive { + const dataModelDirectives = getAllModelDirectiveFromElements(element); + if (dataModelDirectives.length > 0) { + return dataModelDirectives[0]; } if (element.getAttribute('name')) { diff --git a/src/LiveComponent/assets/src/live_controller.ts b/src/LiveComponent/assets/src/live_controller.ts index caf79650aeb..e7c9bb1e8fa 100644 --- a/src/LiveComponent/assets/src/live_controller.ts +++ b/src/LiveComponent/assets/src/live_controller.ts @@ -1,12 +1,15 @@ import { Controller } from '@hotwired/stimulus'; -import { parseDirectives, DirectiveModifier } from './directives_parser'; +import { + parseDirectives, + DirectiveModifier, +} from './Directive/directives_parser'; import { getModelDirectiveFromElement, getElementAsTagText, getValueFromElement, - elementBelongsToThisComponent, + elementBelongsToThisComponent, getAllModelDirectiveFromElements, } from './dom_utils'; -import Component, {proxifyComponent} from './Component'; +import Component, { proxifyComponent } from './Component'; import Backend from './Backend'; import { StandardElementDriver } from './Component/ElementDriver'; import LoadingPlugin from './Component/plugins/LoadingPlugin'; @@ -15,6 +18,7 @@ import PageUnloadingPlugin from './Component/plugins/PageUnloadingPlugin'; import PollingPlugin from './Component/plugins/PollingPlugin'; import SetValueOntoModelFieldsPlugin from './Component/plugins/SetValueOntoModelFieldsPlugin'; import {PluginInterface} from './Component/plugins/PluginInterface'; +import getModelBinding from './Directive/get_model_binding'; export interface LiveEvent extends CustomEvent { detail: { @@ -255,70 +259,44 @@ export default class extends Controller implements LiveController { return; } - let shouldRender = true; - let targetEventName = 'input'; - let debounce: number|boolean = false; - - modelDirective.modifiers.forEach((modifier) => { - switch (modifier.name) { - case 'on': - if (!modifier.value) { - throw new Error(`The "on" modifier in ${modelDirective.getString()} requires a value - e.g. on(change).`); - } - if (!['input', 'change'].includes(modifier.value)) { - throw new Error(`The "on" modifier in ${modelDirective.getString()} only accepts the arguments "input" or "change".`); - } - - targetEventName = modifier.value; - - break; - case 'norender': - shouldRender = false; - - break; - - case 'debounce': - debounce = modifier.value ? parseInt(modifier.value) : true; - - break; - default: - console.warn(`Unknown modifier "${modifier.name}" in data-model="${modelDirective.getString()}".`); - } - }); + const modelBinding = getModelBinding(modelDirective); + if (!modelBinding.targetEventName) { + modelBinding.targetEventName = 'input'; + } // rare case where the same event+element triggers a model // update *and* an action. The action is already scheduled // to occur, so we do not need to *also* trigger a re-render. if (this.pendingActionTriggerModelElement === element) { - shouldRender = false; + modelBinding.shouldRender = false; } // just in case, if a "change" event is happening, and this field // targets "input", set the model to be safe. This helps when people // manually trigger field updates by dispatching a "change" event - if (eventName === 'change' && targetEventName === 'input') { - targetEventName = 'change'; + if (eventName === 'change' && modelBinding.targetEventName === 'input') { + modelBinding.targetEventName = 'change'; } // e.g. we are targeting "change" and this is the "input" event // so do *not* update the model yet - if (eventName && targetEventName !== eventName) { + if (eventName && modelBinding.targetEventName !== eventName) { return; } - if (false === debounce) { - if (targetEventName === 'input') { + if (false === modelBinding.debounce) { + if (modelBinding.targetEventName === 'input') { // true debounce will cause default to be used - debounce = true; + modelBinding.debounce = true; } else { // for change, add no debounce - debounce = 0; + modelBinding.debounce = 0; } } const finalValue = getValueFromElement(element, this.component.valueStore); - this.component.set(modelDirective.action, finalValue, shouldRender, debounce); + this.component.set(modelBinding.modelName, finalValue, modelBinding.shouldRender, modelBinding.debounce); } handleConnectedControllerEvent(event: LiveEvent) { @@ -332,7 +310,13 @@ export default class extends Controller implements LiveController { return; } - this.component.addChild(childController.component); + const modelDirectives = getAllModelDirectiveFromElements(childController.element); + const modelBindings = modelDirectives.map(getModelBinding); + + this.component.addChild( + childController.component, + modelBindings + ); // live:disconnect needs to be registered on the child element directly // that's because if the child component is removed from the DOM, then diff --git a/src/LiveComponent/assets/test/directives_parser.test.ts b/src/LiveComponent/assets/test/Directive/directives_parser.test.ts similarity index 98% rename from src/LiveComponent/assets/test/directives_parser.test.ts rename to src/LiveComponent/assets/test/Directive/directives_parser.test.ts index b79f7add4e0..ececf9a972e 100644 --- a/src/LiveComponent/assets/test/directives_parser.test.ts +++ b/src/LiveComponent/assets/test/Directive/directives_parser.test.ts @@ -1,4 +1,4 @@ -import {Directive, parseDirectives} from '../src/directives_parser'; +import {Directive, parseDirectives} from '../../src/Directive/directives_parser'; const assertDirectiveEquals = function(actual: Directive, expected: any) { // normalize this so that it doesn't trip up the comparison diff --git a/src/LiveComponent/assets/test/Directive/get_model_binding.test.ts b/src/LiveComponent/assets/test/Directive/get_model_binding.test.ts new file mode 100644 index 00000000000..b251cc75641 --- /dev/null +++ b/src/LiveComponent/assets/test/Directive/get_model_binding.test.ts @@ -0,0 +1,37 @@ +import getModelBinding from '../../src/Directive/get_model_binding'; +import {parseDirectives} from '../../src/Directive/directives_parser'; + +describe('get_model_binding', () => { + it('returns correctly with simple directive', () => { + const directive = parseDirectives('firstName')[0]; + expect(getModelBinding(directive)).toEqual({ + modelName: 'firstName', + innerModelName: null, + shouldRender: true, + debounce: false, + targetEventName: null, + }); + }); + + it('returns all modifiers correctly', () => { + const directive = parseDirectives('on(change)|norender|debounce(100)|firstName')[0]; + expect(getModelBinding(directive)).toEqual({ + modelName: 'firstName', + innerModelName: null, + shouldRender: false, + debounce: 100, + targetEventName: 'change', + }); + }); + + it('parses the parent:inner model name correctly', () => { + const directive = parseDirectives('firstName:first')[0]; + expect(getModelBinding(directive)).toEqual({ + modelName: 'firstName', + innerModelName: 'first', + shouldRender: true, + debounce: false, + targetEventName: null, + }); + }); +}); diff --git a/src/LiveComponent/assets/test/controller/child-model.test.ts b/src/LiveComponent/assets/test/controller/child-model.test.ts new file mode 100644 index 00000000000..21094e0a979 --- /dev/null +++ b/src/LiveComponent/assets/test/controller/child-model.test.ts @@ -0,0 +1,140 @@ +/* + * This file is part of the Symfony package. + * + * (c) Fabien Potencier + * + * For the full copyright and license information, please view the LICENSE + * file that was distributed with this source code. + */ + +'use strict'; + +import { createTest, initComponent, shutdownTests } from '../tools'; +import {getByTestId, waitFor} from '@testing-library/dom'; +import userEvent from '@testing-library/user-event'; + +describe('Component parent -> child data-model binding tests', () => { + afterEach(() => { + shutdownTests(); + }) + + // updating stops when child is removed, restarts after + // more complex foo:bar model binding works + // multiple model bindings work + + it('updates parent model in simple setup', async () => { + const test = await createTest({ foodName: ''}, (data: any) => ` +
+ Food Name ${data.foodName} +
+ +
+
+ `); + + test.expectsAjaxCall('get') + .expectSentData({ foodName: 'ice cream' }) + .init(); + + // type into the child component + await userEvent.type(test.queryByDataModel('value'), 'ice cream'); + + // wait for parent to start/stop loading + await waitFor(() => expect(test.element).toHaveAttribute('busy')); + await waitFor(() => expect(test.element).toHaveTextContent('Food Name ice cream')); + }); + + it('will default to "value" for the model name', async () => { + const test = await createTest({ foodName: ''}, (data: any) => ` +
+ Food Name ${data.foodName} +
+ +
+
+ `); + + test.expectsAjaxCall('get') + .expectSentData({ foodName: 'ice cream' }) + .init(); + + // type into the child component + await userEvent.type(test.queryByDataModel('value'), 'ice cream'); + + // wait for parent to start/stop loading + await waitFor(() => expect(test.element).toHaveAttribute('busy')); + await waitFor(() => expect(test.element).toHaveTextContent('Food Name ice cream')); + }); + + it('considers modifiers when updating parent model', async () => { + const test = await createTest({ foodName: ''}, (data: any) => ` +
+ Food Name ${data.foodName} +
+ +
+
+ `); + + // type into the child component + await userEvent.type(test.queryByDataModel('value'), 'ice cream'); + + // wait for parent model to be set + await waitFor(() => expect(test.component.getData('foodName')).toEqual('ice cream')); + // but it never triggers an Ajax call, because the norender modifier + expect(test.element).not.toHaveAttribute('busy'); + // wait for a potential Ajax call to start + await (new Promise(resolve => setTimeout(resolve, 50))); + expect(test.element).not.toHaveAttribute('busy'); + }); + + it('start and stops model binding as child is added/removed', async () => { + // TODO - finish this test! + const test = await createTest({ foodName: ''}, (data: any) => ` +
+ Food Name ${data.foodName} +
+ +
+
+ `); + + test.expectsAjaxCall('get') + .expectSentData({ foodName: 'ice cream' }) + .init(); + + // type into the child component + const inputElement = test.queryByDataModel('value'); + await userEvent.type(inputElement, 'ice cream'); + + // wait for parent to start/stop loading + await waitFor(() => expect(test.element).toHaveAttribute('busy')); + await waitFor(() => expect(test.element).toHaveTextContent('Food Name ice cream')); + + // remove child component + const otherContainer = document.createElement('div'); + otherContainer.appendChild(getByTestId(test.element, 'child')); + + // type into the child component + await userEvent.type(inputElement, ' sandwhich'); + // wait for a potential Ajax call to start + await (new Promise(resolve => setTimeout(resolve, 50))); + expect(test.element).not.toHaveAttribute('busy'); + }); +}); diff --git a/src/LiveComponent/assets/test/controller/child.test.ts b/src/LiveComponent/assets/test/controller/child.test.ts index 96d746b400e..e85b14271e5 100644 --- a/src/LiveComponent/assets/test/controller/child.test.ts +++ b/src/LiveComponent/assets/test/controller/child.test.ts @@ -9,31 +9,11 @@ 'use strict'; -import { - createTestForExistingComponent, - createTest, - initComponent, - shutdownTests -} from '../tools'; -import {getByTestId, waitFor} from '@testing-library/dom'; -import Component from '../../src/Component'; +import { createTestForExistingComponent, createTest, initComponent, shutdownTests, getComponent } from '../tools'; +import { getByTestId, waitFor } from '@testing-library/dom'; import userEvent from '@testing-library/user-event'; -const getComponent = function(element: HTMLElement|null) { - if (!element) { - throw new Error('could not find element'); - } - - // @ts-ignore - const component = element.__component; - if (!(component instanceof Component)) { - throw new Error('missing component'); - } - - return component; -} - -describe('LiveController parent -> child component tests', () => { +describe('Component parent -> child initialization and rendering tests', () => { afterEach(() => { shutdownTests(); }) diff --git a/src/LiveComponent/assets/test/controller/model.test.ts b/src/LiveComponent/assets/test/controller/model.test.ts index 5ff1b1359cd..949d9d006c9 100644 --- a/src/LiveComponent/assets/test/controller/model.test.ts +++ b/src/LiveComponent/assets/test/controller/model.test.ts @@ -142,7 +142,7 @@ describe('LiveController data-model Tests', () => { `); test.expectsAjaxCall('get') - .expectSentData({ color: `orange` }) + .expectSentData({ color: 'orange' }) .init(); await userEvent.type(test.queryByNameAttribute('color'), 'orange'); diff --git a/src/LiveComponent/assets/test/dom_utils.test.ts b/src/LiveComponent/assets/test/dom_utils.test.ts index 176eb79daeb..815e41f3d8c 100644 --- a/src/LiveComponent/assets/test/dom_utils.test.ts +++ b/src/LiveComponent/assets/test/dom_utils.test.ts @@ -8,9 +8,9 @@ import { setValueOnElement } from '../src/dom_utils'; import ValueStore from '../src/Component/ValueStore'; -import Component from "../src/Component"; -import Backend from "../src/Backend"; -import {StandardElementDriver} from "../src/Component/ElementDriver"; +import Component from '../src/Component'; +import Backend from '../src/Backend'; +import {StandardElementDriver} from '../src/Component/ElementDriver'; const createStore = function(props: any = {}): ValueStore { return new ValueStore(props, {}); diff --git a/src/LiveComponent/assets/test/tools.ts b/src/LiveComponent/assets/test/tools.ts index 8657a005c33..6d9f7c3a14b 100644 --- a/src/LiveComponent/assets/test/tools.ts +++ b/src/LiveComponent/assets/test/tools.ts @@ -103,12 +103,22 @@ class FunctionalTest { } queryByDataModel(modelName: string): HTMLElement { - const element = this.element.querySelector(`[data-model$="${modelName}"]`); - if (!element) { + const elements = this.element.querySelectorAll(`[data-model$="${modelName}"]`); + let matchedElement: null|Element = null; + + // skip any elements that are actually controllers + // these are child component bindings, not real fields + elements.forEach((element) => { + if (!element.hasAttribute('data-controller')) { + matchedElement = element; + } + }); + + if (!matchedElement) { throw new Error(`Could not find element with data-model="${modelName}" inside ${this.element.outerHTML}`); } - return element as HTMLElement; + return matchedElement as HTMLElement; } queryByNameAttribute(modelName: string): HTMLElement { @@ -445,3 +455,17 @@ export function initComponent(data: any, props: any = {}, controllerValues: any ${controllerValues.fingerprint ? `data-live-fingerprint-value="${controllerValues.fingerprint}"` : ''} `; } + +export function getComponent(element: HTMLElement|null) { + if (!element) { + throw new Error('could not find element'); + } + + // @ts-ignore + const component = element.__component; + if (!(component instanceof Component)) { + throw new Error('missing component'); + } + + return component; +} diff --git a/src/LiveComponent/src/Resources/doc/index.rst b/src/LiveComponent/src/Resources/doc/index.rst index 10330d9612d..5bb41d72fd2 100644 --- a/src/LiveComponent/src/Resources/doc/index.rst +++ b/src/LiveComponent/src/Resources/doc/index.rst @@ -1799,49 +1799,74 @@ action in the *child* component only, even if the ``save`` action actually only exists in the parent. The same is true for ``data-model``, though there is some special handling for this case (see next point). -If a child model updates, it will attempt to update the parent model -~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ +Updating a Parent Model from a Child +~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ Suppose a child component has a: .. code-block:: html - diff --git a/src/TwigComponent/tests/Integration/EventListener/DataModelPropsSubscriberTest.php b/src/TwigComponent/tests/Integration/EventListener/DataModelPropsSubscriberTest.php new file mode 100644 index 00000000000..5493e26e831 --- /dev/null +++ b/src/TwigComponent/tests/Integration/EventListener/DataModelPropsSubscriberTest.php @@ -0,0 +1,33 @@ + + * + * For the full copyright and license information, please view the LICENSE + * file that was distributed with this source code. + */ + +namespace Symfony\UX\TwigComponent\Tests\Integration; + +use Symfony\Bundle\FrameworkBundle\Test\KernelTestCase; +use Symfony\UX\TwigComponent\ComponentRenderer; + +final class DataModelPropsSubscriberTest extends KernelTestCase +{ + public function testDataModelPropsAreSharedToChild(): void + { + /** @var ComponentRenderer $renderer */ + $renderer = self::getContainer()->get('ux.twig_component.component_renderer'); + + $html = $renderer->createAndRender('parent_form_component', [ + // content is mapped down to "value" in a child component + 'content' => 'Hello data-model!', + 'content2' => 'Value for second child', + ]); + + $this->assertStringContainsString('', $html); + $this->assertStringContainsString('', $html); + } +} diff --git a/src/TwigComponent/tests/Unit/Util/ModelBindingParserTest.php b/src/TwigComponent/tests/Unit/Util/ModelBindingParserTest.php new file mode 100644 index 00000000000..cc951fbfb6d --- /dev/null +++ b/src/TwigComponent/tests/Unit/Util/ModelBindingParserTest.php @@ -0,0 +1,54 @@ + + * + * For the full copyright and license information, please view the LICENSE + * file that was distributed with this source code. + */ + +namespace Symfony\UX\TwigComponent\Tests\Unit\Util; + +use PHPUnit\Framework\TestCase; +use Symfony\UX\TwigComponent\Util\ModelBindingParser; + +final class ModelBindingParserTest extends TestCase +{ + /** + * @dataProvider getModelStringTests + */ + public function testParseAllValidStrings(string $input, array $expectedBindings): void + { + $parser = new ModelBindingParser(); + $this->assertEquals($expectedBindings, $parser->parse($input)); + } + + public function getModelStringTests(): \Generator + { + yield 'empty_string' => ['', []]; + + yield 'valid_but_empty_second_mode' => ['foo:bar ', [ + ['child' => 'foo', 'parent' => 'bar'], + ]]; + + yield 'valid_without_colon_uses_value_default' => ['foo ', [ + ['child' => 'value', 'parent' => 'foo'], + ]]; + + yield 'multiple_spaces_between_models' => ['foo bar:baz', [ + ['child' => 'value', 'parent' => 'foo'], + ['child' => 'bar', 'parent' => 'baz'], + ]]; + } + + public function testParseThrowsExceptionWithMutipleColons(): void + { + $this->expectException(\InvalidArgumentException::class); + $this->expectExceptionMessage('Invalid value "foo:bar:baz" given for "data-model"'); + + $parser = new ModelBindingParser(); + $parser->parse('foo:bar:baz'); + } +} diff --git a/src/Vue/Resources/assets/test/render_controller.test.ts b/src/Vue/Resources/assets/test/render_controller.test.ts index a43ab520dd4..904dd591aa4 100644 --- a/src/Vue/Resources/assets/test/render_controller.test.ts +++ b/src/Vue/Resources/assets/test/render_controller.test.ts @@ -34,7 +34,7 @@ const startStimulus = () => { }; const Hello = { - template: "

Hello {{ name ?? 'world' }}

", + template: '

Hello {{ name ?? \'world\' }}

', props: ['name'] };