Skip to content

Commit df1fd60

Browse files
vaadin-botvursen
andauthored
fix: preserve items added directly to grid cache before attaching (#12035) (#12044)
This PR cherry-picks changes from the original PR #12035 to branch 25.2. --- #### Original PR description > The Grid web component currently discards items that are added directly to the cache before it's attached to the DOM. This is a problem for gridConnector, which relies on adding items this way: for example, when a Grid is rendered inside another grid's ComponentRenderer, the connector populates the cache before the grid gets attached. > > The discarding happens because DataProviderController clears its cache whenever `setPageSize` or `setDataProvider` is called, even when the web component itself doesn't call `clearCache`. This PR removes that logic: besides it causing issues like the one described above, it's more flexible to leave it up to the component to decide when and where to clear the cache after setting a new data provider or page size. > > Removing the automatic clearing has one side effect to deal with. The cache used to receive its pageSize only at creation: when the page size changed, the cache was simply recreated. That no longer happens, so the PR moves pageSize to Cache itself: setting it now updates all sub-caches and cancels their pending page requests. Note that it's important to cancel pending requests because a page requested with the old pageSize could otherwise resolve to the wrong item index. > > Part of vaadin/flow-components#9674 > Co-authored-by: Sergey Vinogradov <mr.vursen@gmail.com>
1 parent 34d1a5b commit df1fd60

5 files changed

Lines changed: 97 additions & 40 deletions

File tree

packages/component-base/src/data-provider-controller/cache.js

Lines changed: 31 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -15,13 +15,6 @@ export class Cache {
1515
*/
1616
context;
1717

18-
/**
19-
* The number of items to display per page.
20-
*
21-
* @type {number}
22-
*/
23-
pageSize;
24-
2518
/**
2619
* An array of cached items.
2720
*
@@ -50,6 +43,14 @@ export class Cache {
5043
*/
5144
#subCacheByIndex = {};
5245

46+
/**
47+
* The number of items per page.
48+
*
49+
* @type {number}
50+
* @private
51+
*/
52+
#pageSize;
53+
5354
/**
5455
* The number of items.
5556
*
@@ -123,6 +124,29 @@ export class Cache {
123124
return this.#flatSize;
124125
}
125126

127+
/**
128+
* The number of items per page.
129+
*
130+
* @return {number}
131+
*/
132+
get pageSize() {
133+
return this.#pageSize;
134+
}
135+
136+
/**
137+
* Sets the number of items per page for this cache and its descendants.
138+
* Changing the page size discards all pending page requests.
139+
*
140+
* @param {number} pageSize
141+
*/
142+
set pageSize(pageSize) {
143+
this.#pageSize = pageSize;
144+
this.pendingRequests = {};
145+
this.subCaches.forEach((subCache) => {
146+
subCache.pageSize = pageSize;
147+
});
148+
}
149+
126150
/**
127151
* The number of items.
128152
*

packages/component-base/src/data-provider-controller/data-provider-controller.js

Lines changed: 14 additions & 17 deletions
Original file line numberDiff line numberDiff line change
@@ -29,13 +29,6 @@ export class DataProviderController extends EventTarget {
2929
*/
3030
dataProviderParams;
3131

32-
/**
33-
* A number of items to display per page.
34-
*
35-
* @type {number}
36-
*/
37-
pageSize;
38-
3932
/**
4033
* A callback that returns whether the given item is expanded.
4134
*
@@ -78,14 +71,13 @@ export class DataProviderController extends EventTarget {
7871
) {
7972
super();
8073
this.host = host;
81-
this.pageSize = pageSize;
8274
this.getItemId = getItemId;
8375
this.isExpanded = isExpanded;
8476
this.placeholder = placeholder;
8577
this.isPlaceholder = isPlaceholder;
8678
this.dataProvider = dataProvider;
8779
this.dataProviderParams = dataProviderParams;
88-
this.rootCache = this.#createRootCache(size);
80+
this.rootCache = this.#createRootCache(pageSize, size);
8981
}
9082

9183
/**
@@ -95,6 +87,13 @@ export class DataProviderController extends EventTarget {
9587
return this.rootCache.flatSize;
9688
}
9789

90+
/**
91+
* The number of items per page in the root cache.
92+
*/
93+
get pageSize() {
94+
return this.rootCache.pageSize;
95+
}
96+
9897
/** @private */
9998
get #cacheContext() {
10099
return {
@@ -113,13 +112,12 @@ export class DataProviderController extends EventTarget {
113112
}
114113

115114
/**
116-
* Sets the page size and clears the cache.
115+
* Sets the number of items per page in the root cache and any of its descendants.
117116
*
118117
* @param {number} pageSize
119118
*/
120119
setPageSize(pageSize) {
121-
this.pageSize = pageSize;
122-
this.clearCache();
120+
this.rootCache.pageSize = pageSize;
123121
}
124122

125123
/**
@@ -129,7 +127,6 @@ export class DataProviderController extends EventTarget {
129127
*/
130128
setDataProvider(dataProvider) {
131129
this.dataProvider = dataProvider;
132-
this.clearCache();
133130
}
134131

135132
/**
@@ -143,7 +140,7 @@ export class DataProviderController extends EventTarget {
143140
* Clears the cache.
144141
*/
145142
clearCache() {
146-
this.rootCache = this.#createRootCache(this.rootCache.size);
143+
this.rootCache = this.#createRootCache(this.rootCache.pageSize, this.rootCache.size);
147144
}
148145

149146
/**
@@ -238,8 +235,8 @@ export class DataProviderController extends EventTarget {
238235
}
239236

240237
/** @private */
241-
#createRootCache(size) {
242-
return new Cache(this.#cacheContext, this.pageSize, size);
238+
#createRootCache(pageSize, size) {
239+
return new Cache(this.#cacheContext, pageSize, size);
243240
}
244241

245242
/** @private */
@@ -250,7 +247,7 @@ export class DataProviderController extends EventTarget {
250247

251248
let params = {
252249
page,
253-
pageSize: this.pageSize,
250+
pageSize: cache.pageSize,
254251
parentItem: cache.parentItem,
255252
};
256253

packages/component-base/test/data-provider-controller-cache.test.js

Lines changed: 32 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -109,6 +109,38 @@ describe('DataProviderController - Cache', () => {
109109
});
110110
});
111111

112+
describe('pageSize', () => {
113+
let subCache;
114+
115+
beforeEach(() => {
116+
cache = new Cache({ isExpanded }, 50, 500);
117+
cache.setPage(0, ['Item 0']);
118+
subCache = cache.createSubCache(0);
119+
});
120+
121+
it('should set the new pageSize', () => {
122+
cache.pageSize = 10;
123+
expect(cache.pageSize).to.equal(10);
124+
});
125+
126+
it('should discard pending requests', () => {
127+
cache.pendingRequests[0] = (_items, _size) => {};
128+
cache.pageSize = 10;
129+
expect(cache.pendingRequests).to.eql({});
130+
});
131+
132+
it('should propagate the new pageSize to sub-caches', () => {
133+
cache.pageSize = 10;
134+
expect(subCache.pageSize).to.equal(10);
135+
});
136+
137+
it('should discard pending requests of sub-caches', () => {
138+
subCache.pendingRequests[0] = (_items, _size) => {};
139+
cache.pageSize = 10;
140+
expect(subCache.pendingRequests).to.eql({});
141+
});
142+
});
143+
112144
describe('subCaches', () => {
113145
let subCache0, subCache1;
114146

packages/component-base/test/data-provider-controller.test.js

Lines changed: 0 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -175,12 +175,6 @@ describe('DataProviderController', () => {
175175
expect(controller.rootCache.pageSize).to.equal(10);
176176
});
177177

178-
it('should clear the cache', () => {
179-
const spy = sinon.spy(controller, 'clearCache');
180-
controller.setPageSize(10);
181-
expect(spy.calledOnce).to.be.true;
182-
});
183-
184178
it('should pass the new pageSize to dataProvider when requesting data', () => {
185179
const dataProviderSpy = sinon.spy(controller, 'dataProvider');
186180
controller.setPageSize(10);
@@ -205,12 +199,6 @@ describe('DataProviderController', () => {
205199
expect(controller.dataProvider).to.equal(dataProvider);
206200
});
207201

208-
it('should clear the cache', () => {
209-
const spy = sinon.spy(controller, 'clearCache');
210-
controller.setDataProvider((_params, callback) => callback([], 0));
211-
expect(spy.calledOnce).to.be.true;
212-
});
213-
214202
it('should request data using the new dataProvider', () => {
215203
controller.setDataProvider((_params, callback) => callback([], 0));
216204
const dataProviderSpy = sinon.spy(controller, 'dataProvider');

packages/grid/test/data-provider.test.js

Lines changed: 20 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -665,12 +665,21 @@ describe('data provider', () => {
665665
});
666666

667667
describe('attached', () => {
668-
it('should have rows when attached and shown after cache is cleared on hidden grid', async () => {
669-
const grid = document.createElement('vaadin-grid');
668+
let grid;
669+
670+
beforeEach(() => {
671+
grid = document.createElement('vaadin-grid');
672+
670673
const col = document.createElement('vaadin-grid-column');
671674
col.setAttribute('path', 'item');
672675
grid.appendChild(col);
676+
});
673677

678+
afterEach(() => {
679+
grid.remove();
680+
});
681+
682+
it('should have rows when attached and shown after cache is cleared on hidden grid', async () => {
674683
grid.size = 1;
675684
grid.dataProvider = function (_params, callback) {
676685
callback([{ item: 'A' }]);
@@ -684,9 +693,16 @@ describe('attached', () => {
684693
grid.clearCache();
685694
grid.removeAttribute('style');
686695
expect(getCellContent(getFirstVisibleItem(grid)).textContent).to.equal('A');
696+
});
687697

688-
// Grid should be removed after test as was attached to body.
689-
document.body.removeChild(grid);
698+
it('should preserve items added directly to cache before attaching', async () => {
699+
grid.size = 1;
700+
grid.dataProvider = (_params, callback) => callback([]);
701+
grid._dataProviderController.rootCache.items[0] = { item: 'A' };
702+
grid._hasData = true;
703+
document.body.appendChild(grid);
704+
await aTimeout(0);
705+
expect(getCellContent(getFirstVisibleItem(grid)).textContent).to.equal('A');
690706
});
691707
});
692708

0 commit comments

Comments
 (0)