Skip to content

Commit 7b54778

Browse files
squrioussqurious
committed
Simplifies mapping and fix some edge cases
1 parent 1809b26 commit 7b54778

File tree

11 files changed

+104
-45
lines changed

11 files changed

+104
-45
lines changed

‎src/LiveComponent/assets/dist/Component/plugins/QueryStringPlugin.d.ts

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -9,5 +9,6 @@ export default class implements PluginInterface {
99
[p: string]: QueryMapping;
1010
});
1111
attachToComponent(component: Component): void;
12+
private isEmpty;
1213
}
1314
export {};

‎src/LiveComponent/assets/dist/live_controller.js

Lines changed: 29 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -2707,11 +2707,14 @@ function toQueryString(data) {
27072707
Object.entries(data).forEach(([iKey, iValue]) => {
27082708
const key = baseKey === '' ? iKey : `${baseKey}[${iKey}]`;
27092709
if (!isObject(iValue)) {
2710-
if (iValue !== null) {
2710+
if (null !== iValue) {
27112711
entries[key] = encodeURIComponent(iValue)
27122712
.replace(/%20/g, '+')
27132713
.replace(/%2C/g, ',');
27142714
}
2715+
else if ('' === baseKey) {
2716+
entries[key] = '';
2717+
}
27152718
}
27162719
else {
27172720
entries = Object.assign(Object.assign({}, entries), buildQueryStringEntries(iValue, entries, key));
@@ -2731,22 +2734,22 @@ function fromQueryString(search) {
27312734
const insertDotNotatedValueIntoData = (key, value, data) => {
27322735
const [first, second, ...rest] = key.split('.');
27332736
if (!second)
2734-
return (data[key] = value);
2737+
return data[key] = value;
27352738
if (data[first] === undefined) {
2736-
data[first] = Number.isNaN(second) ? {} : [];
2739+
data[first] = Number.isNaN(Number.parseInt(second)) ? {} : [];
27372740
}
27382741
insertDotNotatedValueIntoData([second, ...rest].join('.'), value, data[first]);
27392742
};
27402743
const entries = search.split('&').map((i) => i.split('='));
27412744
const data = {};
27422745
entries.forEach(([key, value]) => {
2743-
if (!value)
2744-
return;
27452746
value = decodeURIComponent(value.replace(/\+/g, '%20'));
27462747
if (!key.includes('[')) {
27472748
data[key] = value;
27482749
}
27492750
else {
2751+
if ('' === value)
2752+
return;
27502753
const dotNotatedKey = key.replace(/\[/g, '.').replace(/]/g, '');
27512754
insertDotNotatedValueIntoData(dotNotatedKey, value, data);
27522755
}
@@ -2796,13 +2799,33 @@ class QueryStringPlugin {
27962799
const urlUtils = new UrlUtils(window.location.href);
27972800
const currentUrl = urlUtils.toString();
27982801
Object.entries(this.mapping).forEach(([prop, mapping]) => {
2799-
urlUtils.set(mapping.name, component.valueStore.get(prop));
2802+
const value = component.valueStore.get(prop);
2803+
if (this.isEmpty(value)) {
2804+
urlUtils.remove(mapping.name);
2805+
}
2806+
else {
2807+
urlUtils.set(mapping.name, value);
2808+
}
28002809
});
28012810
if (currentUrl !== urlUtils.toString()) {
28022811
HistoryStrategy.replace(urlUtils);
28032812
}
28042813
});
28052814
}
2815+
isEmpty(value) {
2816+
if (null === value || value === '' || value === undefined || Array.isArray(value) && value.length === 0) {
2817+
return true;
2818+
}
2819+
if (typeof value !== 'object') {
2820+
return false;
2821+
}
2822+
for (let key of Object.keys(value)) {
2823+
if (!this.isEmpty(value[key])) {
2824+
return false;
2825+
}
2826+
}
2827+
return true;
2828+
}
28062829
}
28072830

28082831
const getComponent = (element) => LiveControllerDefault.componentRegistry.getComponent(element);

‎src/LiveComponent/assets/src/Component/plugins/QueryStringPlugin.ts

Lines changed: 24 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -18,7 +18,12 @@ export default class implements PluginInterface {
1818
const currentUrl = urlUtils.toString();
1919

2020
Object.entries(this.mapping).forEach(([prop, mapping]) => {
21-
urlUtils.set(mapping.name, component.valueStore.get(prop));
21+
const value = component.valueStore.get(prop);
22+
if (this.isEmpty(value)) {
23+
urlUtils.remove(mapping.name);
24+
} else {
25+
urlUtils.set(mapping.name, value);
26+
}
2227
});
2328

2429
// Only update URL if it has changed
@@ -27,4 +32,22 @@ export default class implements PluginInterface {
2732
}
2833
});
2934
}
35+
36+
private isEmpty(value: any): boolean
37+
{
38+
if (null === value || value === '' || value === undefined || Array.isArray(value) && value.length === 0) {
39+
return true;
40+
}
41+
42+
if (typeof value !== 'object') {
43+
return false;
44+
}
45+
46+
for (let key of Object.keys(value)) {
47+
if (!this.isEmpty(value[key])) {
48+
return false;
49+
}
50+
}
51+
return true;
52+
}
3053
}

‎src/LiveComponent/assets/src/url_utils.ts

Lines changed: 9 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -15,10 +15,13 @@ function toQueryString(data: any) {
1515
const key = baseKey === '' ? iKey : `${baseKey}[${iKey}]`;
1616

1717
if (!isObject(iValue)) {
18-
if (iValue !== null) {
18+
if (null !== iValue) {
1919
entries[key] = encodeURIComponent(iValue)
2020
.replace(/%20/g, '+') // Conform to RFC1738
2121
.replace(/%2C/g, ',');
22+
} else if ('' === baseKey) {
23+
// Keep empty values for top level data
24+
entries[key] = '';
2225
}
2326
} else {
2427
entries = { ...entries, ...buildQueryStringEntries(iValue, entries, key) };
@@ -51,11 +54,11 @@ function fromQueryString(search: string) {
5154
const [first, second, ...rest] = key.split('.');
5255

5356
// We're at a leaf node, let's make the assigment...
54-
if (!second) return (data[key] = value);
57+
if (!second) return data[key] = value;
5558

5659
// This is where we fill in empty arrays/objects along the way to the assigment...
5760
if (data[first] === undefined) {
58-
data[first] = Number.isNaN(second) ? {} : [];
61+
data[first] = Number.isNaN(Number.parseInt(second)) ? {} : [];
5962
}
6063

6164
// Keep deferring assignment until the full key is built up...
@@ -67,14 +70,14 @@ function fromQueryString(search: string) {
6770
const data: any = {};
6871

6972
entries.forEach(([key, value]) => {
70-
// Query string params don't always have values... (`?foo=`)
71-
if (!value) return;
72-
7373
value = decodeURIComponent(value.replace(/\+/g, '%20'));
7474

7575
if (!key.includes('[')) {
7676
data[key] = value;
7777
} else {
78+
// Skip empty nested data
79+
if ('' === value) return;
80+
7881
// Convert to dot notation because it's easier...
7982
const dotNotatedKey = key.replace(/\[/g, '.').replace(/]/g, '');
8083

‎src/LiveComponent/assets/test/url_utils.test.ts

Lines changed: 27 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -35,12 +35,24 @@ describe('url_utils', () => {
3535
expect(urlUtils.search).toEqual('?param=bar');
3636
});
3737

38+
it('preserve empty values if the param is scalar', () => {
39+
urlUtils.set('param', '');
40+
41+
expect(urlUtils.search).toEqual('?param=');
42+
});
43+
3844
it('expand arrays in the URL', () => {
3945
urlUtils.set('param', ['foo', 'bar']);
4046

4147
expect(urlUtils.search).toEqual('?param[0]=foo&param[1]=bar');
4248
});
4349

50+
it('prevent empty values if the param is array', () => {
51+
urlUtils.set('param', []);
52+
53+
expect(urlUtils.search).toEqual('');
54+
});
55+
4456
it('expand objects in the URL', () => {
4557
urlUtils.set('param', {
4658
foo: 1,
@@ -49,6 +61,21 @@ describe('url_utils', () => {
4961

5062
expect(urlUtils.search).toEqual('?param[foo]=1&param[bar]=baz');
5163
});
64+
65+
it('remove empty values in nested object properties', () => {
66+
urlUtils.set('param', {
67+
foo: null,
68+
bar: 'baz',
69+
});
70+
71+
expect(urlUtils.search).toEqual('?param[bar]=baz');
72+
});
73+
74+
it('prevent empty values if the param is an empty object', () => {
75+
urlUtils.set('param', {});
76+
77+
expect(urlUtils.search).toEqual('');
78+
});
5279
});
5380

5481
describe('remove', () => {

‎src/LiveComponent/src/EventListener/QueryStringInitializeSubscriber.php

Lines changed: 0 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -27,11 +27,6 @@
2727
*/
2828
class QueryStringInitializeSubscriber implements EventSubscriberInterface
2929
{
30-
/**
31-
* @var array<class-string,LiveComponentMetadata>
32-
*/
33-
private array $registered = [];
34-
3530
public function __construct(
3631
private readonly RequestStack $requestStack,
3732
private readonly LiveComponentMetadataFactory $metadataFactory,

‎src/LiveComponent/src/Metadata/LiveComponentMetadataFactory.php

Lines changed: 2 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -136,15 +136,9 @@ private function createQueryStringMapping(string $propertyName, LiveProp $livePr
136136
return [];
137137
}
138138

139-
$queryStringMapping = [];
140-
141-
$queryStringMapping['parameters'] = [
142-
$propertyName => [
143-
'property' => $propertyName,
144-
],
139+
return [
140+
'name' => $propertyName,
145141
];
146-
147-
return $queryStringMapping;
148142
}
149143

150144
public function reset(): void

‎src/LiveComponent/src/Metadata/LivePropMetadata.php

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -55,7 +55,7 @@ public function allowsNull(): bool
5555
}
5656

5757
/**
58-
* @return array{'parameters': array<string,array{'property': string}>}
58+
* @return array{'name': string}
5959
*/
6060
public function getQueryStringMapping(): array
6161
{

‎src/LiveComponent/src/Util/LiveControllerAttributesCreator.php

Lines changed: 1 addition & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -126,11 +126,7 @@ public function attributesForRendering(MountedComponent $mounted, ComponentMetad
126126
$queryMapping = [];
127127
foreach ($liveMetadata->getAllLivePropsMetadata() as $livePropMetadata) {
128128
if ($mapping = $livePropMetadata->getQueryStringMapping()) {
129-
foreach ($mapping['parameters'] as $parameter => $config) {
130-
$queryMapping[$config['property']] = [
131-
'name' => $parameter,
132-
];
133-
}
129+
$queryMapping[$livePropMetadata->getName()] = $mapping;
134130
}
135131
}
136132
$attributesCollection->setQueryUrlMapping($queryMapping);

‎src/LiveComponent/src/Util/QueryStringPropsExtractor.php

Lines changed: 3 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -38,13 +38,12 @@ public function extract(Request $request, LiveComponentMetadata $metadata, objec
3838
$data = [];
3939

4040
foreach ($metadata->getAllLivePropsMetadata() as $livePropMetadata) {
41-
$queryStringBinding = $livePropMetadata->getQueryStringMapping();
42-
foreach ($queryStringBinding['parameters'] ?? [] as $parameterName => $paramConfig) {
43-
if (null !== ($value = $query[$parameterName] ?? null)) {
41+
if ($queryStringMapping = $livePropMetadata->getQueryStringMapping()) {
42+
if (null !== ($value = $query[$queryStringMapping['name']] ?? null)) {
4443
if (\is_array($value) && $this->isNumericIndexedArray($value)) {
4544
ksort($value);
4645
}
47-
$data[$paramConfig['property']] = $this->hydrator->hydrateValue($value, $livePropMetadata, $component);
46+
$data[$livePropMetadata->getName()] = $this->hydrator->hydrateValue($value, $livePropMetadata, $component);
4847
}
4948
}
5049
}

‎src/LiveComponent/tests/Functional/Metadata/LiveComponentMetadataFactoryTest.php

Lines changed: 7 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -31,23 +31,21 @@ public function testQueryStringMapping()
3131
}
3232

3333
$this->assertEquals([
34-
'parameters' => [
35-
'prop1' => ['property' => 'prop1'],
36-
],
34+
'name' => 'prop1',
3735
], $propsMetadataByName['prop1']->getQueryStringMapping());
3836

3937
$this->assertEquals([
40-
'parameters' => [
41-
'prop2' => ['property' => 'prop2'],
42-
],
38+
'name' => 'prop2',
4339
], $propsMetadataByName['prop2']->getQueryStringMapping());
4440

4541
$this->assertEquals([
46-
'parameters' => [
47-
'prop3' => ['property' => 'prop3'],
48-
],
42+
'name' => 'prop3',
4943
], $propsMetadataByName['prop3']->getQueryStringMapping());
5044

5145
$this->assertEquals([], $propsMetadataByName['prop4']->getQueryStringMapping());
46+
47+
$this->assertEquals([
48+
'name' => 'prop5',
49+
], $propsMetadataByName['prop5']->getQueryStringMapping());
5250
}
5351
}

0 commit comments

Comments
 (0)