From a29beee22da0885d11fd1a064123127c52d69f3e Mon Sep 17 00:00:00 2001 From: Ryan Weaver Date: Fri, 26 Jan 2024 16:07:49 -0500 Subject: [PATCH] More work on data-event to use action params --- src/LiveComponent/CHANGELOG.md | 5 ++ .../assets/src/live_controller.ts | 30 ++++---- .../test/Directive/directives_parser.test.ts | 70 +------------------ .../assets/test/controller/action.test.ts | 6 +- .../assets/test/controller/emit.test.ts | 6 +- src/LiveComponent/doc/index.rst | 8 +-- 6 files changed, 32 insertions(+), 93 deletions(-) diff --git a/src/LiveComponent/CHANGELOG.md b/src/LiveComponent/CHANGELOG.md index 240eeb0fe3d..8d292848e29 100644 --- a/src/LiveComponent/CHANGELOG.md +++ b/src/LiveComponent/CHANGELOG.md @@ -41,6 +41,11 @@ >Save ``` +- [BC BREAK] The `data-event` attribute was removed in favor of using Stimulus + "action parameters": rename `data-event` to `data-live-event-param`. Additionally, + if you were passing arguments to the event name, use action parameter attributes + for those as well - e.g. `data-live-foo-param="bar"`. + ## 2.15.0 - [BC BREAK] The `data-live-id` attribute was changed to `id`. diff --git a/src/LiveComponent/assets/src/live_controller.ts b/src/LiveComponent/assets/src/live_controller.ts index af02e6cc7c2..6cd03118799 100644 --- a/src/LiveComponent/assets/src/live_controller.ts +++ b/src/LiveComponent/assets/src/live_controller.ts @@ -187,28 +187,28 @@ export default class LiveControllerDefault extends Controller imple }); } - emit(event: Event) { + $render() { + return this.component.render(); + } + + emit(event: any) { this.getEmitDirectives(event).forEach(({ name, data, nameMatch }) => { this.component.emit(name, data, nameMatch); }); } - emitUp(event: Event) { + emitUp(event: any) { this.getEmitDirectives(event).forEach(({ name, data, nameMatch }) => { this.component.emitUp(name, data, nameMatch); }); } - emitSelf(event: Event) { + emitSelf(event: any) { this.getEmitDirectives(event).forEach(({ name, data }) => { this.component.emitSelf(name, data); }); } - $render() { - return this.component.render(); - } - /** * Update a model value. * @@ -229,13 +229,15 @@ export default class LiveControllerDefault extends Controller imple this.component.fingerprint = this.fingerprintValue; } - private getEmitDirectives(event: Event): Array<{ name: string; data: any; nameMatch: string | null }> { - const element = event.currentTarget as HTMLElement; - if (!element.dataset.event) { - throw new Error(`No data-event attribute found on element: ${getElementAsTagText(element)}`); + private getEmitDirectives(event: any): Array<{ name: string; data: any; nameMatch: string | null }> { + const params = event.params; + if (!params.action) { + throw new Error(`No event name provided on element: ${getElementAsTagText(event.currentTarget)}. Did you forget to add the "data-live-event-param" attribute?`); } - - const eventInfo = element.dataset.event; + const eventInfo = params.event; + // all other params are considered event arguments + const eventArgs = { ...params }; + delete eventArgs.event; // data-event="name(product_list)|some_event" const directives = parseDirectives(eventInfo); @@ -254,7 +256,7 @@ export default class LiveControllerDefault extends Controller imple emits.push({ name: directive.action, - data: directive.named, + data: eventArgs, nameMatch, }); }); diff --git a/src/LiveComponent/assets/test/Directive/directives_parser.test.ts b/src/LiveComponent/assets/test/Directive/directives_parser.test.ts index ececf9a972e..8c13c25d4ed 100644 --- a/src/LiveComponent/assets/test/Directive/directives_parser.test.ts +++ b/src/LiveComponent/assets/test/Directive/directives_parser.test.ts @@ -29,7 +29,6 @@ describe('directives parser', () => { assertDirectiveEquals(directives[0], { action: 'hide', args: [], - named: {}, modifiers: [], }) }); @@ -40,7 +39,6 @@ describe('directives parser', () => { assertDirectiveEquals(directives[0], { action: 'addClass', args: ['opacity-50'], - named: {}, modifiers: [], }) }); @@ -51,7 +49,6 @@ describe('directives parser', () => { assertDirectiveEquals(directives[0], { action: 'addClass', args: ['opacity-50 disabled'], - named: {}, modifiers: [], }) }); @@ -63,7 +60,6 @@ describe('directives parser', () => { action: 'addClass', // space between arguments is trimmed args: ['opacity-50', 'disabled'], - named: {}, modifiers: [], }) }); @@ -74,13 +70,11 @@ describe('directives parser', () => { assertDirectiveEquals(directives[0], { action: 'addClass', args: ['opacity-50'], - named: {}, modifiers: [], }) assertDirectiveEquals(directives[1], { action: 'addAttribute', args: ['disabled'], - named: {}, modifiers: [], }) }); @@ -91,63 +85,16 @@ describe('directives parser', () => { assertDirectiveEquals(directives[0], { action: 'hide', args: [], - named: {}, modifiers: [], }) assertDirectiveEquals(directives[1], { action: 'addClass', args: ['opacity-50 disabled'], - named: {}, modifiers: [], }) assertDirectiveEquals(directives[2], { action: 'addAttribute', args: ['disabled'], - named: {}, - modifiers: [], - }) - }); - - it('parses single named argument', () => { - const directives = parseDirectives('save(foo=bar)'); - expect(directives).toHaveLength(1); - assertDirectiveEquals(directives[0], { - action: 'save', - args: [], - named: { foo: 'bar' }, - modifiers: [], - }) - }); - - it('parses multiple named arguments', () => { - const directives = parseDirectives('save(foo=bar, baz=bazzles)'); - expect(directives).toHaveLength(1); - assertDirectiveEquals(directives[0], { - action: 'save', - args: [], - named: { foo: 'bar', baz: 'bazzles' }, - modifiers: [], - }) - }); - - it('parses arguments and spaces are kept', () => { - const directives = parseDirectives('save(foo= bar)'); - expect(directives).toHaveLength(1); - assertDirectiveEquals(directives[0], { - action: 'save', - args: [], - named: { foo: ' bar' }, - modifiers: [], - }) - }); - - it('parses argument names with space is trimmed', () => { - const directives = parseDirectives('save(foo =bar)'); - expect(directives).toHaveLength(1); - assertDirectiveEquals(directives[0], { - action: 'save', - args: [], - named: { foo: 'bar' }, modifiers: [], }) }); @@ -158,7 +105,6 @@ describe('directives parser', () => { assertDirectiveEquals(directives[0], { action: 'addClass', args: ['disabled'], - named: {}, modifiers: [ { name: 'delay', value: null } ], @@ -171,7 +117,6 @@ describe('directives parser', () => { assertDirectiveEquals(directives[0], { action: 'addClass', args: ['disabled'], - named: {}, modifiers: [ { name: 'delay', value: '400' }, ], @@ -179,12 +124,11 @@ describe('directives parser', () => { }); it('parses multiple modifiers', () => { - const directives = parseDirectives('prevent|debounce(400)|save(foo=bar)'); + const directives = parseDirectives('prevent|debounce(400)|save'); expect(directives).toHaveLength(1); assertDirectiveEquals(directives[0], { action: 'save', args: [], - named: { foo: 'bar' }, modifiers: [ { name: 'prevent', value: null }, { name: 'debounce', value: '400' }, @@ -211,22 +155,10 @@ describe('directives parser', () => { }).toThrow('Missing space after addClass()') }); - it('named and unnamed arguments cannot be mixed', () => { - expect(() => { - parseDirectives('save(foo=bar, baz)'); - }).toThrow('Normal and named arguments cannot be mixed inside "save()"') - }); - it('modifier cannot have multiple arguments', () => { expect(() => { parseDirectives('debounce(10, 20)|save'); }).toThrow('The modifier "debounce()" does not support multiple arguments.') }); - - it('modifier cannot have named arguments', () => { - expect(() => { - parseDirectives('debounce(foo=bar)|save'); - }).toThrow('The modifier "debounce()" does not support named arguments.') - }); }); }); diff --git a/src/LiveComponent/assets/test/controller/action.test.ts b/src/LiveComponent/assets/test/controller/action.test.ts index db8f6d3663b..d4f7ba9ea24 100644 --- a/src/LiveComponent/assets/test/controller/action.test.ts +++ b/src/LiveComponent/assets/test/controller/action.test.ts @@ -81,14 +81,14 @@ describe('LiveController Action Tests', () => { data-live-action-param="sendNamedArgs" data-live-a-param="1" data-live-b-param="2" - data-live-c-param="3" + data-live-c-param="banana" >Send named args `); // ONLY a post is sent, not a re-render GET test.expectsAjaxCall() - .expectActionCalled('sendNamedArgs', {a: '1', b: '2', c: '3'}) + .expectActionCalled('sendNamedArgs', {a: 1, b: 2, c: 'banana'}) .serverWillChangeProps((data: any) => { // server marks component as "saved" data.isSaved = true; @@ -185,7 +185,7 @@ describe('LiveController Action Tests', () => { test.expectsAjaxCall() // 3 actions called .expectActionCalled('save') - .expectActionCalled('sync', { syncAll: '1' }) + .expectActionCalled('sync', { syncAll: 1 }) .expectActionCalled('save') .serverWillChangeProps((data: any) => { data.isSaved = true; diff --git a/src/LiveComponent/assets/test/controller/emit.test.ts b/src/LiveComponent/assets/test/controller/emit.test.ts index 15f739ada6e..69f0adb3c8b 100644 --- a/src/LiveComponent/assets/test/controller/emit.test.ts +++ b/src/LiveComponent/assets/test/controller/emit.test.ts @@ -26,17 +26,17 @@ describe('LiveController Emit Tests', () => { Render Count: ${data.renderCount} `); diff --git a/src/LiveComponent/doc/index.rst b/src/LiveComponent/doc/index.rst index 657638b0b5f..eeb9f8d6dcb 100644 --- a/src/LiveComponent/doc/index.rst +++ b/src/LiveComponent/doc/index.rst @@ -2462,7 +2462,7 @@ There are three ways to emit an event: