Skip to content

Commit 06e026c

Browse files
javiereguiluzjaviereguiluz
committed
minor #2180 [StimulusBundle] Improve StimulusAttributes rendering performances by switching to html escaping strategy (Kocal)
This PR was squashed before being merged into the 2.x branch. Discussion ---------- [StimulusBundle] Improve `StimulusAttributes` rendering performances by switching to `html` escaping strategy | Q | A | ------------- | --- | Bug fix? | no | New feature? | no <!-- please update src/**/CHANGELOG.md files --> | Issues | Fix #... <!-- prefix each issue number with "Fix #", no need to create an issue if none exist, explain below instead --> | License | MIT <!-- Replace this notice by a description of your feature/bugfix. This will help reviewers and should be a good start for the documentation. Additionally (see https://symfony.com/releases): - Always add tests and ensure they pass. - For new features, provide some code snippets to help understand usage. - Features and deprecations must be submitted against branch main. - Changelog entry should follow https://symfony.com/doc/current/contributing/code/conventions.html#writing-a-changelog-entry - Never break backward compatibility (see https://symfony.com/bc). --> A better alternative to #2178, by changing the escaping strategy for HTML attributes value from `html_attr` to `html`, as indicated by `@stof` in twigphp/Twig#4322 (comment). On my use case (a `Map` with 1000 `Marker` and `InfoWindow`), [I have a performance gain of ~68%](https://blackfire.io/profiles/compare/56f4d7d2-ee56-487f-8a78-420f4037165f/graph): <img width="1057" alt="image" src="https://github.com/user-attachments/assets/8b7ff48d-8e6e-4dae-b09e-2c323bc2449d"> This PR should also improve performances of our packages using the `StimulusAttributes` DTO, like Chart.js, LiveComponent, ... --- > [!IMPORTANT] > The initial PR changed a bit, the default rendering strategy does not change, but instead we introduce a new configuration to use the new (and optimized) rendering strategy, see #2180 (comment). Commits ------- 647532a [StimulusBundle] Improve `StimulusAttributes` rendering performances by switching to `html` escaping strategy
2 parents 99085b4 + 647532a commit 06e026c

File tree

10 files changed

+45
-65
lines changed

10 files changed

+45
-65
lines changed

‎src/Chartjs/tests/Twig/ChartExtensionTest.php

+1-1
Original file line numberDiff line numberDiff line change
@@ -56,7 +56,7 @@ public function testRenderChart()
5656
);
5757

5858
$this->assertSame(
59-
'<canvas data-controller="mycontroller symfony--ux-chartjs--chart" data-symfony--ux-chartjs--chart-view-value="&#x7B;&quot;type&quot;&#x3A;&quot;line&quot;,&quot;data&quot;&#x3A;&#x7B;&quot;labels&quot;&#x3A;&#x5B;&quot;January&quot;,&quot;February&quot;,&quot;March&quot;,&quot;April&quot;,&quot;May&quot;,&quot;June&quot;,&quot;July&quot;&#x5D;,&quot;datasets&quot;&#x3A;&#x5B;&#x7B;&quot;label&quot;&#x3A;&quot;My&#x20;First&#x20;dataset&quot;,&quot;backgroundColor&quot;&#x3A;&quot;rgb&#x28;255,&#x20;99,&#x20;132&#x29;&quot;,&quot;borderColor&quot;&#x3A;&quot;rgb&#x28;255,&#x20;99,&#x20;132&#x29;&quot;,&quot;data&quot;&#x3A;&#x5B;0,10,5,2,20,30,45&#x5D;&#x7D;&#x5D;&#x7D;,&quot;options&quot;&#x3A;&#x7B;&quot;showLines&quot;&#x3A;false&#x7D;&#x7D;" class="myclass"></canvas>',
59+
'<canvas data-controller="mycontroller symfony--ux-chartjs--chart" data-symfony--ux-chartjs--chart-view-value="{&quot;type&quot;:&quot;line&quot;,&quot;data&quot;:{&quot;labels&quot;:[&quot;January&quot;,&quot;February&quot;,&quot;March&quot;,&quot;April&quot;,&quot;May&quot;,&quot;June&quot;,&quot;July&quot;],&quot;datasets&quot;:[{&quot;label&quot;:&quot;My First dataset&quot;,&quot;backgroundColor&quot;:&quot;rgb(255, 99, 132)&quot;,&quot;borderColor&quot;:&quot;rgb(255, 99, 132)&quot;,&quot;data&quot;:[0,10,5,2,20,30,45]}]},&quot;options&quot;:{&quot;showLines&quot;:false}}" class="myclass"></canvas>',
6060
$rendered
6161
);
6262
}

‎src/LiveComponent/tests/Unit/Twig/LiveComponentRuntimeTest.php

+4-6
Original file line numberDiff line numberDiff line change
@@ -28,17 +28,15 @@ public function testGetLiveAction(): void
2828
$this->assertSame('data-action="live#action" data-live-prop1-param="val1" data-live-some-prop-param="val2" data-live-action-param="action-name"', $props);
2929

3030
$props = $runtime->liveAction('action-name', ['prop1' => 'val1', 'prop2' => 'val2'], ['debounce' => 300]);
31-
$this->assertSame('data-action="live#action" data-live-prop1-param="val1" data-live-prop2-param="val2" data-live-action-param="debounce(300)|action-name"', html_entity_decode($props));
32-
$this->assertSame('data-action="live#action" data-live-prop1-param="val1" data-live-prop2-param="val2" data-live-action-param="debounce&#x28;300&#x29;&#x7C;action-name"', $props);
31+
$this->assertSame('data-action="live#action" data-live-prop1-param="val1" data-live-prop2-param="val2" data-live-action-param="debounce(300)|action-name"', $props);
3332

3433
$props = $runtime->liveAction('action-name:prevent', ['pro1' => 'val1', 'prop2' => 'val2'], ['debounce' => 300]);
35-
$this->assertSame('data-action="live#action:prevent" data-live-pro1-param="val1" data-live-prop2-param="val2" data-live-action-param="debounce(300)|action-name"', html_entity_decode($props));
36-
$this->assertSame('data-action="live#action&#x3A;prevent" data-live-pro1-param="val1" data-live-prop2-param="val2" data-live-action-param="debounce&#x28;300&#x29;&#x7C;action-name"', $props);
34+
$this->assertSame('data-action="live#action:prevent" data-live-pro1-param="val1" data-live-prop2-param="val2" data-live-action-param="debounce(300)|action-name"', $props);
3735

3836
$props = $runtime->liveAction('action-name:prevent', [], ['debounce' => 300]);
39-
$this->assertSame('data-action="live#action:prevent" data-live-action-param="debounce(300)|action-name"', html_entity_decode($props));
37+
$this->assertSame('data-action="live#action:prevent" data-live-action-param="debounce(300)|action-name"', $props);
4038

4139
$props = $runtime->liveAction('action-name', [], [], 'keydown.esc');
42-
$this->assertSame('data-action="keydown.esc->live#action" data-live-action-param="action-name"', html_entity_decode($props));
40+
$this->assertSame('data-action="keydown.esc->live#action" data-live-action-param="action-name"', $props);
4341
}
4442
}

‎src/Notify/tests/Twig/NotifyRuntimeTest.php

+4-4
Original file line numberDiff line numberDiff line change
@@ -39,13 +39,13 @@ public function testStreamNotifications(array $params, string $expected)
3939

4040
public static function streamNotificationsDataProvider(): iterable
4141
{
42-
$publicUrl = 'http&#x3A;&#x2F;&#x2F;localhost&#x3A;9090&#x2F;.well-known&#x2F;mercure';
42+
$publicUrl = 'http://localhost:9090/.well-known/mercure';
4343

4444
yield [
4545
[['/topic/1', '/topic/2']],
4646
'<div '.
4747
'data-controller="symfony--ux-notify--notify" '.
48-
'data-symfony--ux-notify--notify-topics-value="&#x5B;&quot;&#x5C;&#x2F;topic&#x5C;&#x2F;1&quot;,&quot;&#x5C;&#x2F;topic&#x5C;&#x2F;2&quot;&#x5D;" '.
48+
'data-symfony--ux-notify--notify-topics-value="[&quot;\/topic\/1&quot;,&quot;\/topic\/2&quot;]" '.
4949
'data-symfony--ux-notify--notify-hub-value="'.$publicUrl.'"'.
5050
'></div>',
5151
];
@@ -54,7 +54,7 @@ public static function streamNotificationsDataProvider(): iterable
5454
['/topic/1'],
5555
'<div '.
5656
'data-controller="symfony--ux-notify--notify" '.
57-
'data-symfony--ux-notify--notify-topics-value="&#x5B;&quot;&#x5C;&#x2F;topic&#x5C;&#x2F;1&quot;&#x5D;" '.
57+
'data-symfony--ux-notify--notify-topics-value="[&quot;\/topic\/1&quot;]" '.
5858
'data-symfony--ux-notify--notify-hub-value="'.$publicUrl.'"'.
5959
'></div>',
6060
];
@@ -63,7 +63,7 @@ public static function streamNotificationsDataProvider(): iterable
6363
[],
6464
'<div '.
6565
'data-controller="symfony--ux-notify--notify" '.
66-
'data-symfony--ux-notify--notify-topics-value="&#x5B;&quot;https&#x3A;&#x5C;&#x2F;&#x5C;&#x2F;symfony.com&#x5C;&#x2F;notifier&quot;&#x5D;" '.
66+
'data-symfony--ux-notify--notify-topics-value="[&quot;https:\/\/symfony.com\/notifier&quot;]" '.
6767
'data-symfony--ux-notify--notify-hub-value="'.$publicUrl.'"'.
6868
'></div>',
6969
];

‎src/React/tests/Twig/ReactComponentExtensionTest.php

+2-2
Original file line numberDiff line numberDiff line change
@@ -36,7 +36,7 @@ public function testRenderComponent()
3636
);
3737

3838
$this->assertSame(
39-
'data-controller="symfony--ux-react--react" data-symfony--ux-react--react-component-value="SubDir&#x2F;MyComponent" data-symfony--ux-react--react-props-value="&#x7B;&quot;fullName&quot;&#x3A;&quot;Titouan&#x20;Galopin&quot;&#x7D;"',
39+
'data-controller="symfony--ux-react--react" data-symfony--ux-react--react-component-value="SubDir/MyComponent" data-symfony--ux-react--react-props-value="{&quot;fullName&quot;:&quot;Titouan Galopin&quot;}"',
4040
$rendered
4141
);
4242
}
@@ -52,7 +52,7 @@ public function testRenderComponentWithoutProps()
5252
$rendered = $extension->renderReactComponent('SubDir/MyComponent');
5353

5454
$this->assertSame(
55-
'data-controller="symfony--ux-react--react" data-symfony--ux-react--react-component-value="SubDir&#x2F;MyComponent"',
55+
'data-controller="symfony--ux-react--react" data-symfony--ux-react--react-component-value="SubDir/MyComponent"',
5656
$rendered
5757
);
5858
}

‎src/StimulusBundle/src/Dto/StimulusAttributes.php

+25-43
Original file line numberDiff line numberDiff line change
@@ -107,54 +107,36 @@ public function addAttribute(string $name, string $value): void
107107

108108
public function __toString(): string
109109
{
110-
$controllers = array_map(function (string $controllerName): string {
111-
return $this->escapeAsHtmlAttr($controllerName);
112-
}, $this->controllers);
113-
114-
// done separately so we can escape, but avoid escaping ->
115-
$actions = array_map(function (array $actionData): string {
116-
$controllerName = $this->escapeAsHtmlAttr($actionData['controllerName']);
117-
$actionName = $this->escapeAsHtmlAttr($actionData['actionName']);
118-
$eventName = $actionData['eventName'];
119-
120-
$action = $controllerName.'#'.$actionName;
121-
if (null !== $eventName) {
122-
$action = $this->escapeAsHtmlAttr($eventName).'->'.$action;
123-
}
124-
125-
return $action;
126-
}, $this->actions);
110+
$attributes = [];
127111

128-
$targets = [];
129-
foreach ($this->targets as $key => $targetNamesString) {
130-
$targetNames = explode(' ', $targetNamesString);
131-
$targets[$key] = implode(' ', array_map(function (string $targetName): string {
132-
return $this->escapeAsHtmlAttr($targetName);
133-
}, $targetNames));
112+
if ($this->controllers) {
113+
$attributes[] = 'data-controller="'.$this->escape(implode(' ', $this->controllers)).'"';
134114
}
135115

136-
$attributes = [];
116+
if ($this->actions) {
117+
$actions = [];
118+
foreach ($this->actions as ['controllerName' => $controllerName, 'actionName' => $actionName, 'eventName' => $eventName]) {
119+
$action = $this->escape($controllerName.'#'.$actionName);
120+
if (null !== $eventName) {
121+
// done separately so we can escape, but avoid escaping ->
122+
$action = $this->escape($eventName).'->'.$action;
123+
}
124+
125+
$actions[] = $action;
126+
}
137127

138-
if ($controllers) {
139-
$attributes[] = \sprintf('data-controller="%s"', implode(' ', $controllers));
128+
$attributes[] = 'data-action="'.implode(' ', $actions).'"';
140129
}
141130

142-
if ($actions) {
143-
$attributes[] = \sprintf('data-action="%s"', implode(' ', $actions));
131+
foreach ($this->targets as $k => $v) {
132+
$attributes[] = $this->escape($k, 'html_attr').'="'.$this->escape($v).'"';
144133
}
145134

146-
if ($targets) {
147-
$attributes[] = implode(' ', array_map(function (string $key, string $value): string {
148-
return \sprintf('%s="%s"', $key, $value);
149-
}, array_keys($targets), $targets));
135+
foreach ($this->attributes as $k => $v) {
136+
$attributes[] = $this->escape($k, 'html_attr').'="'.$this->escape($v).'"';
150137
}
151138

152-
return rtrim(implode(' ', [
153-
...$attributes,
154-
...array_map(function (string $attribute, string $value): string {
155-
return $attribute.'="'.$this->escapeAsHtmlAttr($value).'"';
156-
}, array_keys($this->attributes), $this->attributes),
157-
]));
139+
return implode(' ', $attributes);
158140
}
159141

160142
public function toArray(): array
@@ -193,7 +175,7 @@ public function toEscapedArray(): array
193175
{
194176
$escaped = [];
195177
foreach ($this->toArray() as $key => $value) {
196-
$escaped[$key] = $this->escapeAsHtmlAttr($value);
178+
$escaped[$key] = $this->escape($value);
197179
}
198180

199181
return $escaped;
@@ -212,18 +194,18 @@ private function getFormattedValue(mixed $value): string
212194
return (string) $value;
213195
}
214196

215-
private function escapeAsHtmlAttr(mixed $value): string
197+
private function escape(mixed $value, string $strategy = 'html'): string
216198
{
217199
if (class_exists(EscaperRuntime::class)) {
218-
return $this->env->getRuntime(EscaperRuntime::class)->escape($value, 'html_attr');
200+
return $this->env->getRuntime(EscaperRuntime::class)->escape($value, $strategy);
219201
}
220202

221203
if (method_exists(EscaperExtension::class, 'escape')) {
222-
return EscaperExtension::escape($this->env, $value, 'html_attr');
204+
return EscaperExtension::escape($this->env, $value, $strategy);
223205
}
224206

225207
// since twig/twig 3.9.0: Using the internal "twig_escape_filter" function is deprecated.
226-
return (string) twig_escape_filter($this->env, $value, 'html_attr');
208+
return (string) twig_escape_filter($this->env, $value, $strategy);
227209
}
228210

229211
/**

‎src/StimulusBundle/tests/Dto/StimulusAttributesTest.php

+1-1
Original file line numberDiff line numberDiff line change
@@ -145,7 +145,7 @@ public function testIsTraversable()
145145
public function testAddAttribute()
146146
{
147147
$this->stimulusAttributes->addAttribute('foo', 'bar baz');
148-
$this->assertSame('foo="bar&#x20;baz"', (string) $this->stimulusAttributes);
148+
$this->assertSame('foo="bar baz"', (string) $this->stimulusAttributes);
149149
$this->assertSame(['foo' => 'bar baz'], $this->stimulusAttributes->toArray());
150150
}
151151
}

‎src/StimulusBundle/tests/Twig/StimulusTwigExtensionTest.php

+2-2
Original file line numberDiff line numberDiff line change
@@ -135,8 +135,8 @@ public function testAppendStimulusController(): void
135135
$extension = new StimulusTwigExtension(new StimulusHelper($this->twig));
136136
$dto = $extension->renderStimulusController('my-controller', ['myValue' => 'scalar-value']);
137137
$this->assertSame(
138-
'data-controller="my-controller another-controller" data-my-controller-my-value-value="scalar-value" data-another-controller-another-value-value="scalar-value&#x20;2"',
139-
(string) $extension->appendStimulusController($dto, 'another-controller', ['another-value' => 'scalar-value 2']),
138+
'data-controller="my-controller another-controller" data-my-controller-my-value-value="scalar-value" data-another-controller-another-value-value="scalar-value 2" data-another-controller-json-value-value="{&quot;key&quot;:&quot;Value with quotes &#039; and \&quot;.&quot;}"',
139+
(string) $extension->appendStimulusController($dto, 'another-controller', ['another-value' => 'scalar-value 2', 'jsonValue' => json_encode(['key' => 'Value with quotes \' and ".'])]),
140140
);
141141
}
142142

‎src/Svelte/tests/Twig/SvelteComponentExtensionTest.php

+3-3
Original file line numberDiff line numberDiff line change
@@ -37,7 +37,7 @@ public function testRenderComponent()
3737
);
3838

3939
$this->assertSame(
40-
'data-controller="symfony--ux-svelte--svelte" data-symfony--ux-svelte--svelte-component-value="SubDir&#x2F;MyComponent" data-symfony--ux-svelte--svelte-props-value="&#x7B;&quot;fullName&quot;&#x3A;&quot;Titouan&#x20;Galopin&quot;&#x7D;"',
40+
'data-controller="symfony--ux-svelte--svelte" data-symfony--ux-svelte--svelte-component-value="SubDir/MyComponent" data-symfony--ux-svelte--svelte-props-value="{&quot;fullName&quot;:&quot;Titouan Galopin&quot;}"',
4141
$rendered
4242
);
4343
}
@@ -53,7 +53,7 @@ public function testRenderComponentWithoutProps()
5353
$rendered = $extension->renderSvelteComponent('SubDir/MyComponent');
5454

5555
$this->assertSame(
56-
'data-controller="symfony--ux-svelte--svelte" data-symfony--ux-svelte--svelte-component-value="SubDir&#x2F;MyComponent"',
56+
'data-controller="symfony--ux-svelte--svelte" data-symfony--ux-svelte--svelte-component-value="SubDir/MyComponent"',
5757
$rendered
5858
);
5959
}
@@ -73,7 +73,7 @@ public function testRenderComponentWithIntro()
7373
);
7474

7575
$this->assertSame(
76-
'data-controller="symfony--ux-svelte--svelte" data-symfony--ux-svelte--svelte-component-value="SubDir&#x2F;MyComponent" data-symfony--ux-svelte--svelte-props-value="&#x7B;&quot;fullName&quot;&#x3A;&quot;Titouan&#x20;Galopin&quot;&#x7D;" data-symfony--ux-svelte--svelte-intro-value="true"',
76+
'data-controller="symfony--ux-svelte--svelte" data-symfony--ux-svelte--svelte-component-value="SubDir/MyComponent" data-symfony--ux-svelte--svelte-props-value="{&quot;fullName&quot;:&quot;Titouan Galopin&quot;}" data-symfony--ux-svelte--svelte-intro-value="true"',
7777
$rendered
7878
);
7979
}

‎src/TwigComponent/tests/Unit/ComponentAttributesTest.php

+1-1
Original file line numberDiff line numberDiff line change
@@ -147,7 +147,7 @@ public function testCanAddStimulusControllerViaStimulusAttributes(): void
147147
'data-controller' => 'foo live',
148148
'data-live-data-value' => '{}',
149149
'data-foo-name-value' => 'ryan',
150-
'data-foo-some-array-value' => '&#x5B;&quot;a&quot;,&quot;b&quot;&#x5D;',
150+
'data-foo-some-array-value' => '[&quot;a&quot;,&quot;b&quot;]',
151151
], $attributes->all());
152152
}
153153

‎src/Vue/tests/Twig/VueComponentExtensionTest.php

+2-2
Original file line numberDiff line numberDiff line change
@@ -37,7 +37,7 @@ public function testRenderComponent()
3737
);
3838

3939
$this->assertSame(
40-
'data-controller="symfony--ux-vue--vue" data-symfony--ux-vue--vue-component-value="SubDir&#x2F;MyComponent" data-symfony--ux-vue--vue-props-value="&#x7B;&quot;fullName&quot;&#x3A;&quot;Titouan&#x20;Galopin&quot;&#x7D;"',
40+
'data-controller="symfony--ux-vue--vue" data-symfony--ux-vue--vue-component-value="SubDir/MyComponent" data-symfony--ux-vue--vue-props-value="{&quot;fullName&quot;:&quot;Titouan Galopin&quot;}"',
4141
$rendered
4242
);
4343
}
@@ -53,7 +53,7 @@ public function testRenderComponentWithoutProps()
5353
$rendered = $extension->renderVueComponent('SubDir/MyComponent');
5454

5555
$this->assertSame(
56-
'data-controller="symfony--ux-vue--vue" data-symfony--ux-vue--vue-component-value="SubDir&#x2F;MyComponent"',
56+
'data-controller="symfony--ux-vue--vue" data-symfony--ux-vue--vue-component-value="SubDir/MyComponent"',
5757
$rendered
5858
);
5959
}

0 commit comments

Comments
 (0)