Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
22 changes: 22 additions & 0 deletions .changeset/7231-gantt-stale-reload-finally.md
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,22 @@
---
'@object-ui/plugin-gantt': patch
---

`ObjectGantt` no longer blanks the chart when one reload supersedes another

`reload()` already sequenced concurrent runs with `reloadSeqRef` and guarded every
result write with `isCurrent()`, but its `finally` was unguarded — so a **superseded**
reload still flipped `loading` / `refreshing` off. The stale run only had to finish
first, which is the ordinary case whenever a second reload is issued while the first
is still in flight: the placeholder was released, no rows had arrived, and the user
saw an empty chart until the fresh response landed.

The `finally` now clears the flags only when the run reaching it is still the current
one. It clears **both** flags rather than only the one its own `silent` mode set:
being current at that point means nothing is in flight any more, so clearing only its
own mode would strand the other flag whenever the superseded run used the other mode
— a silent toolbar refresh overtaken by a filter-change reload would have left the
refresh button busy for the life of the component.

This is the reload guard alone. Nothing about which queries are issued, how they are
projected or how they page changes.
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,231 @@
/**
* ObjectUI
* Copyright (c) 2024-present ObjectStack Inc.
*
* This source code is licensed under the MIT license found in the
* LICENSE file in the root directory of this source tree.
*/

/**
* objectui#7231 — `reload()`'s `finally` must belong to the CURRENT reload.
*
* `reload()` sequences concurrent runs with `reloadSeqRef` and guards every
* result write with `isCurrent()` (`setData` on three branches, `setError` on
* the error branch). The `finally` used to carry no guard, so a SUPERSEDED
* reload still flipped `loading` / `refreshing` off — clearing the loading
* placeholder while the fresh query was still in flight. The user saw an
* empty chart: placeholder gone, no rows arrived yet.
*
* Note which ordering produces it: NOT an exotic out-of-order response, but
* the plain in-issue-order one. The stale reload merely has to FINISH FIRST,
* which is the ordinary case whenever a second reload is issued while the
* first is still in flight. The out-of-order case (fresh finishes first) is
* the one the pre-existing `setData` guard already covered, and it is kept
* below as the control.
*
* The guard shape matters, hence the third case. The flags are per-MODE
* (`silent` → `refreshing`, otherwise `loading`), so a `finally` that clears
* only its own mode's flag when current leaks the other one: a silent reload
* superseded by a non-silent one would never clear `refreshing`, leaving the
* toolbar's refresh button stuck busy for the life of the component. What
* makes clearing BOTH correct is that "I am current AND I am finishing"
* means nothing is in flight any more — a newer reload would have made this
* one stale, and an older one has no claim on the flags.
*
* Scope: this is the reload guard only. The overlapping-reload pairs it
* covers include the toolbar refresh and the write-readback paths, where two
* reloads legitimately overlap and no schema gating is involved — see the
* card for why this must not be folded into the gating work.
*/

import React from 'react';
import { render, screen, waitFor, fireEvent, act } from '@testing-library/react';
import { describe, it, expect, vi } from 'vitest';
import { ObjectGantt } from './ObjectGantt';
import type { DataSource } from '@object-ui/types';

// Probe stand-in: the real chart is irrelevant here, but `refreshing` is not —
// case 3 reads it back off the DOM.
vi.mock('./GanttView', () => ({
GanttView: ({ tasks, onRefresh, refreshing }: any) => (
<div data-testid="gantt-view" data-refreshing={String(!!refreshing)}>
{tasks.map((t: any) => (
<div key={t.id} data-testid="gantt-task">{t.title}</div>
))}
<button data-testid="gv-refresh" onClick={() => onRefresh?.()}>refresh</button>
</div>
),
}));

const PLACEHOLDER = 'Loading Gantt chart...';

const ROWS_A = [
{ id: '1', name: 'From the stale query', start_date: '2024-01-01', end_date: '2024-01-05' },
];
const ROWS_B = [
{ id: '2', name: 'From the fresh query', start_date: '2024-02-01', end_date: '2024-02-05' },
];

const OBJECT_SCHEMA = {
fields: {
name: { type: 'text' },
start_date: { type: 'date' },
end_date: { type: 'date' },
},
};

const GANTT_CONFIG = {
titleField: 'name',
startDateField: 'start_date',
endDateField: 'end_date',
};

function schemaWith(filter?: unknown): any {
return {
type: 'gantt',
gantt: GANTT_CONFIG,
data: { provider: 'object', object: 'tasks' },
...(filter === undefined ? {} : { filter }),
};
}

interface Deferred<T> {
promise: Promise<T>;
resolve: (value: T) => void;
}

function deferred<T>(): Deferred<T> {
let resolve!: (value: T) => void;
const promise = new Promise<T>((res) => {
resolve = res;
});
return { promise, resolve };
}

/**
* A data source whose every `find()` hands back a promise the test resolves
* by hand, so reload N and reload N+1 can be held in flight together and
* completed in either order. `getObjectSchema` resolves immediately — that is
* what issues the second reload (`objectSchema` is a `reload` dependency)
* while the first `find()` is still pending.
*/
function makeDeferredDataSource() {
const finds: Deferred<any>[] = [];
const dataSource = {
find: vi.fn(() => {
const d = deferred<any>();
finds.push(d);
return d.promise;
}),
findOne: vi.fn(),
create: vi.fn(),
update: vi.fn().mockResolvedValue({}),
delete: vi.fn(),
getObjectSchema: vi.fn().mockResolvedValue(OBJECT_SCHEMA),
} as unknown as DataSource;
return { dataSource, finds };
}

/** Settle one held `find()` and let React flush the resulting commits. */
async function settle(d: Deferred<any>, rows: unknown[]) {
await act(async () => {
d.resolve({ data: rows });
await Promise.resolve();
});
}

/** Let pending microtasks/effects run without resolving anything. */
async function flush() {
await act(async () => {
await Promise.resolve();
});
}

describe('ObjectGantt — a superseded reload must not clear the loading state (objectui#7231)', () => {
it('keeps the placeholder up when the STALE reload finishes first and the fresh one is still in flight', async () => {
const { dataSource, finds } = makeDeferredDataSource();

render(<ObjectGantt schema={schemaWith()} dataSource={dataSource} />);

// Reload #1 (mount) is in flight; the object schema resolves and re-keys
// `reload`, issuing reload #2 before #1 has answered.
await waitFor(() => expect((dataSource.find as any).mock.calls.length).toBe(2));
expect(screen.getByText(PLACEHOLDER)).toBeTruthy();

// The superseded reload #1 answers first — the ordinary ordering.
await settle(finds[0], ROWS_A);

// Its `finally` must NOT clear `loading`: the fresh query has not answered,
// so releasing the placeholder here paints an empty chart.
expect(screen.getByText(PLACEHOLDER)).toBeTruthy();
expect(screen.queryByTestId('gantt-view')).toBeNull();

// The current reload #2 answers and owns the transition out of loading.
await settle(finds[1], ROWS_B);

await waitFor(() => expect(screen.getByTestId('gantt-view')).toBeTruthy());
expect(screen.getByText('From the fresh query')).toBeTruthy();
expect(screen.queryByText('From the stale query')).toBeNull();
});

it('control — the fresh reload finishing FIRST paints its rows, and the late stale answer changes nothing', async () => {
const { dataSource, finds } = makeDeferredDataSource();

render(<ObjectGantt schema={schemaWith()} dataSource={dataSource} />);

await waitFor(() => expect((dataSource.find as any).mock.calls.length).toBe(2));

// Out-of-order: the current reload #2 answers before the superseded #1.
await settle(finds[1], ROWS_B);

await waitFor(() => expect(screen.getByTestId('gantt-view')).toBeTruthy());
expect(screen.getByText('From the fresh query')).toBeTruthy();

// The late stale answer must neither clobber the data (the pre-existing
// `setData` guard) nor put the placeholder back.
await settle(finds[0], ROWS_A);
await flush();

expect(screen.getByTestId('gantt-view')).toBeTruthy();
expect(screen.getByText('From the fresh query')).toBeTruthy();
expect(screen.queryByText('From the stale query')).toBeNull();
expect(screen.queryByText(PLACEHOLDER)).toBeNull();
});

it('does not strand `refreshing` when a SILENT reload is superseded by a non-silent one', async () => {
const { dataSource, finds } = makeDeferredDataSource();

const { rerender } = render(<ObjectGantt schema={schemaWith()} dataSource={dataSource} />);

await waitFor(() => expect((dataSource.find as any).mock.calls.length).toBe(2));
await settle(finds[0], ROWS_A);
await settle(finds[1], ROWS_A);
await waitFor(() => expect(screen.getByTestId('gantt-view')).toBeTruthy());
expect(screen.getByTestId('gantt-view').getAttribute('data-refreshing')).toBe('false');

// Toolbar refresh → reload #3, silent: it owns `refreshing`, not `loading`.
fireEvent.click(screen.getByTestId('gv-refresh'));
await waitFor(() =>
expect(screen.getByTestId('gantt-view').getAttribute('data-refreshing')).toBe('true'),
);

// A filter change re-keys `reload` → reload #4, non-silent, superseding the
// silent one while it is still in flight. Different flag, same sequence.
rerender(<ObjectGantt schema={schemaWith({ status: 'open' })} dataSource={dataSource} />);
await waitFor(() => expect((dataSource.find as any).mock.calls.length).toBe(4));
expect(screen.getByText(PLACEHOLDER)).toBeTruthy();

// The superseded silent reload answers: it must touch neither flag.
await settle(finds[2], ROWS_A);
expect(screen.getByText(PLACEHOLDER)).toBeTruthy();

// The current reload answers. Nothing is in flight any more, so BOTH flags
// must be honest — a guard that only cleared `loading` here would leave the
// refresh button spinning forever.
await settle(finds[3], ROWS_B);

await waitFor(() => expect(screen.getByTestId('gantt-view')).toBeTruthy());
expect(screen.getByTestId('gantt-view').getAttribute('data-refreshing')).toBe('false');
expect(screen.getByText('From the fresh query')).toBeTruthy();
});
});
20 changes: 18 additions & 2 deletions packages/plugin-gantt/src/ObjectGantt.tsx
Original file line numberDiff line numberDiff line change
Expand Up@@ -686,8 +686,24 @@ export const ObjectGantt: React.FC<ObjectGanttProps> = ({
setError(err as Error);
}
} finally {
if (silent) setRefreshing(false);
else setLoading(false);
// Only the NEWEST reload owns the loading flags, for the same reason
// the result writes above are guarded. An unguarded clear here let a
// SUPERSEDED reload release the placeholder while the fresh query was
// still in flight, so the chart painted empty in between
// (objectui#7231).
//
// The current reload clears BOTH flags, not just the one its own
// `silent` mode set: reaching this point as the current run means
// nothing is in flight any more — a newer reload would have made this
// one stale, and an older one has no claim on the flags. Clearing only
// this run's own mode would strand the other one whenever the
// superseded reload ran in the OTHER mode (a silent toolbar refresh
// overtaken by a filter-change reload would leave `refreshing` on for
// the life of the component).
if (isCurrent()) {
setRefreshing(false);
setLoading(false);
}
}
// eslint-disable-next-line react-hooks/exhaustive-deps -- (rest as any).data intentionally untracked, matching the original effect
}, [effectiveDataSource, resource, hasInlineData, dataProvider, dataItems, schema.filter, schema.sort, objectSchema]);
Expand Down
Loading
, 'i'); if (__m === '*' || __re.test(location.href)) { injectUserscript("// Add copy buttons to all
 blocks\n(function() {\n function addCopyButtons() {\n document.querySelectorAll('pre code').forEach(function(codeBlock) {\n if (codeBlock.parentElement.hasAttribute('data-copy-added')) return;\n codeBlock.parentElement.setAttribute('data-copy-added', 'true');\n \n var btn = document.createElement('button');\n btn.textContent = 'Copy';\n btn.style.cssText = 'position:absolute;top:4px;right:4px;padding:2px 8px;font-size:11px;background:#4ecdc4;border:none;border-radius:4px;color:#1a1a2e;cursor:pointer;opacity:0.7;transition:opacity 0.2s;';\n btn.onmouseover = function() { this.style.opacity = '1'; };\n btn.onmouseout = function() { this.style.opacity = '0.7'; };\n btn.onclick = function() {\n navigator.clipboard.writeText(codeBlock.textContent).then(function() {\n btn.textContent = 'Copied!';\n setTimeout(function() { btn.textContent = 'Copy'; }, 1500);\n });\n };\n codeBlock.parentElement.style.position = 'relative';\n codeBlock.parentElement.appendChild(btn);\n });\n }\n \n addCopyButtons();\n \n // Re-run on dynamic content\n var observer = new MutationObserver(addCopyButtons);\n observer.observe(document.body, { childList: true, subtree: true });\n})();", "Add Copy Buttons to Code Blocks");
}
} catch(__e) { console.warn('[Userscript:Add Copy Buttons to Code Blocks]', __e); }
})();
(function(){
try {
var __m = "github.com";
var __re = new RegExp('^' + "github\\.com" + '
Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
22 changes: 22 additions & 0 deletions .changeset/7231-gantt-stale-reload-finally.md
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,22 @@
---
'@object-ui/plugin-gantt': patch
---

`ObjectGantt` no longer blanks the chart when one reload supersedes another

`reload()` already sequenced concurrent runs with `reloadSeqRef` and guarded every
result write with `isCurrent()`, but its `finally` was unguarded — so a **superseded**
reload still flipped `loading` / `refreshing` off. The stale run only had to finish
first, which is the ordinary case whenever a second reload is issued while the first
is still in flight: the placeholder was released, no rows had arrived, and the user
saw an empty chart until the fresh response landed.

The `finally` now clears the flags only when the run reaching it is still the current
one. It clears **both** flags rather than only the one its own `silent` mode set:
being current at that point means nothing is in flight any more, so clearing only its
own mode would strand the other flag whenever the superseded run used the other mode
— a silent toolbar refresh overtaken by a filter-change reload would have left the
refresh button busy for the life of the component.

This is the reload guard alone. Nothing about which queries are issued, how they are
projected or how they page changes.
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,231 @@
/**
* ObjectUI
* Copyright (c) 2024-present ObjectStack Inc.
*
* This source code is licensed under the MIT license found in the
* LICENSE file in the root directory of this source tree.
*/

/**
* objectui#7231 — `reload()`'s `finally` must belong to the CURRENT reload.
*
* `reload()` sequences concurrent runs with `reloadSeqRef` and guards every
* result write with `isCurrent()` (`setData` on three branches, `setError` on
* the error branch). The `finally` used to carry no guard, so a SUPERSEDED
* reload still flipped `loading` / `refreshing` off — clearing the loading
* placeholder while the fresh query was still in flight. The user saw an
* empty chart: placeholder gone, no rows arrived yet.
*
* Note which ordering produces it: NOT an exotic out-of-order response, but
* the plain in-issue-order one. The stale reload merely has to FINISH FIRST,
* which is the ordinary case whenever a second reload is issued while the
* first is still in flight. The out-of-order case (fresh finishes first) is
* the one the pre-existing `setData` guard already covered, and it is kept
* below as the control.
*
* The guard shape matters, hence the third case. The flags are per-MODE
* (`silent` → `refreshing`, otherwise `loading`), so a `finally` that clears
* only its own mode's flag when current leaks the other one: a silent reload
* superseded by a non-silent one would never clear `refreshing`, leaving the
* toolbar's refresh button stuck busy for the life of the component. What
* makes clearing BOTH correct is that "I am current AND I am finishing"
* means nothing is in flight any more — a newer reload would have made this
* one stale, and an older one has no claim on the flags.
*
* Scope: this is the reload guard only. The overlapping-reload pairs it
* covers include the toolbar refresh and the write-readback paths, where two
* reloads legitimately overlap and no schema gating is involved — see the
* card for why this must not be folded into the gating work.
*/

import React from 'react';
import { render, screen, waitFor, fireEvent, act } from '@testing-library/react';
import { describe, it, expect, vi } from 'vitest';
import { ObjectGantt } from './ObjectGantt';
import type { DataSource } from '@object-ui/types';

// Probe stand-in: the real chart is irrelevant here, but `refreshing` is not —
// case 3 reads it back off the DOM.
vi.mock('./GanttView', () => ({
GanttView: ({ tasks, onRefresh, refreshing }: any) => (
<div data-testid="gantt-view" data-refreshing={String(!!refreshing)}>
{tasks.map((t: any) => (
<div key={t.id} data-testid="gantt-task">{t.title}</div>
))}
<button data-testid="gv-refresh" onClick={() => onRefresh?.()}>refresh</button>
</div>
),
}));

const PLACEHOLDER = 'Loading Gantt chart...';

const ROWS_A = [
{ id: '1', name: 'From the stale query', start_date: '2024-01-01', end_date: '2024-01-05' },
];
const ROWS_B = [
{ id: '2', name: 'From the fresh query', start_date: '2024-02-01', end_date: '2024-02-05' },
];

const OBJECT_SCHEMA = {
fields: {
name: { type: 'text' },
start_date: { type: 'date' },
end_date: { type: 'date' },
},
};

const GANTT_CONFIG = {
titleField: 'name',
startDateField: 'start_date',
endDateField: 'end_date',
};

function schemaWith(filter?: unknown): any {
return {
type: 'gantt',
gantt: GANTT_CONFIG,
data: { provider: 'object', object: 'tasks' },
...(filter === undefined ? {} : { filter }),
};
}

interface Deferred<T> {
promise: Promise<T>;
resolve: (value: T) => void;
}

function deferred<T>(): Deferred<T> {
let resolve!: (value: T) => void;
const promise = new Promise<T>((res) => {
resolve = res;
});
return { promise, resolve };
}

/**
* A data source whose every `find()` hands back a promise the test resolves
* by hand, so reload N and reload N+1 can be held in flight together and
* completed in either order. `getObjectSchema` resolves immediately — that is
* what issues the second reload (`objectSchema` is a `reload` dependency)
* while the first `find()` is still pending.
*/
function makeDeferredDataSource() {
const finds: Deferred<any>[] = [];
const dataSource = {
find: vi.fn(() => {
const d = deferred<any>();
finds.push(d);
return d.promise;
}),
findOne: vi.fn(),
create: vi.fn(),
update: vi.fn().mockResolvedValue({}),
delete: vi.fn(),
getObjectSchema: vi.fn().mockResolvedValue(OBJECT_SCHEMA),
} as unknown as DataSource;
return { dataSource, finds };
}

/** Settle one held `find()` and let React flush the resulting commits. */
async function settle(d: Deferred<any>, rows: unknown[]) {
await act(async () => {
d.resolve({ data: rows });
await Promise.resolve();
});
}

/** Let pending microtasks/effects run without resolving anything. */
async function flush() {
await act(async () => {
await Promise.resolve();
});
}

describe('ObjectGantt — a superseded reload must not clear the loading state (objectui#7231)', () => {
it('keeps the placeholder up when the STALE reload finishes first and the fresh one is still in flight', async () => {
const { dataSource, finds } = makeDeferredDataSource();

render(<ObjectGantt schema={schemaWith()} dataSource={dataSource} />);

// Reload #1 (mount) is in flight; the object schema resolves and re-keys
// `reload`, issuing reload #2 before #1 has answered.
await waitFor(() => expect((dataSource.find as any).mock.calls.length).toBe(2));
expect(screen.getByText(PLACEHOLDER)).toBeTruthy();

// The superseded reload #1 answers first — the ordinary ordering.
await settle(finds[0], ROWS_A);

// Its `finally` must NOT clear `loading`: the fresh query has not answered,
// so releasing the placeholder here paints an empty chart.
expect(screen.getByText(PLACEHOLDER)).toBeTruthy();
expect(screen.queryByTestId('gantt-view')).toBeNull();

// The current reload #2 answers and owns the transition out of loading.
await settle(finds[1], ROWS_B);

await waitFor(() => expect(screen.getByTestId('gantt-view')).toBeTruthy());
expect(screen.getByText('From the fresh query')).toBeTruthy();
expect(screen.queryByText('From the stale query')).toBeNull();
});

it('control — the fresh reload finishing FIRST paints its rows, and the late stale answer changes nothing', async () => {
const { dataSource, finds } = makeDeferredDataSource();

render(<ObjectGantt schema={schemaWith()} dataSource={dataSource} />);

await waitFor(() => expect((dataSource.find as any).mock.calls.length).toBe(2));

// Out-of-order: the current reload #2 answers before the superseded #1.
await settle(finds[1], ROWS_B);

await waitFor(() => expect(screen.getByTestId('gantt-view')).toBeTruthy());
expect(screen.getByText('From the fresh query')).toBeTruthy();

// The late stale answer must neither clobber the data (the pre-existing
// `setData` guard) nor put the placeholder back.
await settle(finds[0], ROWS_A);
await flush();

expect(screen.getByTestId('gantt-view')).toBeTruthy();
expect(screen.getByText('From the fresh query')).toBeTruthy();
expect(screen.queryByText('From the stale query')).toBeNull();
expect(screen.queryByText(PLACEHOLDER)).toBeNull();
});

it('does not strand `refreshing` when a SILENT reload is superseded by a non-silent one', async () => {
const { dataSource, finds } = makeDeferredDataSource();

const { rerender } = render(<ObjectGantt schema={schemaWith()} dataSource={dataSource} />);

await waitFor(() => expect((dataSource.find as any).mock.calls.length).toBe(2));
await settle(finds[0], ROWS_A);
await settle(finds[1], ROWS_A);
await waitFor(() => expect(screen.getByTestId('gantt-view')).toBeTruthy());
expect(screen.getByTestId('gantt-view').getAttribute('data-refreshing')).toBe('false');

// Toolbar refresh → reload #3, silent: it owns `refreshing`, not `loading`.
fireEvent.click(screen.getByTestId('gv-refresh'));
await waitFor(() =>
expect(screen.getByTestId('gantt-view').getAttribute('data-refreshing')).toBe('true'),
);

// A filter change re-keys `reload` → reload #4, non-silent, superseding the
// silent one while it is still in flight. Different flag, same sequence.
rerender(<ObjectGantt schema={schemaWith({ status: 'open' })} dataSource={dataSource} />);
await waitFor(() => expect((dataSource.find as any).mock.calls.length).toBe(4));
expect(screen.getByText(PLACEHOLDER)).toBeTruthy();

// The superseded silent reload answers: it must touch neither flag.
await settle(finds[2], ROWS_A);
expect(screen.getByText(PLACEHOLDER)).toBeTruthy();

// The current reload answers. Nothing is in flight any more, so BOTH flags
// must be honest — a guard that only cleared `loading` here would leave the
// refresh button spinning forever.
await settle(finds[3], ROWS_B);

await waitFor(() => expect(screen.getByTestId('gantt-view')).toBeTruthy());
expect(screen.getByTestId('gantt-view').getAttribute('data-refreshing')).toBe('false');
expect(screen.getByText('From the fresh query')).toBeTruthy();
});
});
20 changes: 18 additions & 2 deletions packages/plugin-gantt/src/ObjectGantt.tsx
Original file line numberDiff line numberDiff line change
Expand Up@@ -686,8 +686,24 @@ export const ObjectGantt: React.FC<ObjectGanttProps> = ({
setError(err as Error);
}
} finally {
if (silent) setRefreshing(false);
else setLoading(false);
// Only the NEWEST reload owns the loading flags, for the same reason
// the result writes above are guarded. An unguarded clear here let a
// SUPERSEDED reload release the placeholder while the fresh query was
// still in flight, so the chart painted empty in between
// (objectui#7231).
//
// The current reload clears BOTH flags, not just the one its own
// `silent` mode set: reaching this point as the current run means
// nothing is in flight any more — a newer reload would have made this
// one stale, and an older one has no claim on the flags. Clearing only
// this run's own mode would strand the other one whenever the
// superseded reload ran in the OTHER mode (a silent toolbar refresh
// overtaken by a filter-change reload would leave `refreshing` on for
// the life of the component).
if (isCurrent()) {
setRefreshing(false);
setLoading(false);
}
}
// eslint-disable-next-line react-hooks/exhaustive-deps -- (rest as any).data intentionally untracked, matching the original effect
}, [effectiveDataSource, resource, hasInlineData, dataProvider, dataItems, schema.filter, schema.sort, objectSchema]);
Expand Down
Loading
, 'i'); if (__m === '*' || __re.test(location.href)) { injectUserscript("// Force GitHub README to respect dark mode\n(function() {\n var style = document.createElement('style');\n style.textContent = '\n .markdown-body {\n color-scheme: dark light;\n }\n .markdown-body pre { background: #161b22 !important; }\n .markdown-body code { background: rgba(110, 118, 129, 0.4) !important; }\n .markdown-body table th, .markdown-body table td { border-color: #30363d !important; }\n .markdown-body img { background: #0d1117; }\n .markdown-body blockquote { border-left-color: #8b949e; }\n .markdown-body hr { border-color: #30363d; }\n ';\n document.head.appendChild(style);\n})();", "GitHub Dark Mode README Fix"); } } catch(__e) { console.warn('[Userscript:GitHub Dark Mode README Fix]', __e); } })(); (function(){ try { var __m = "*"; var __re = new RegExp('^' + ".*" + '
Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
22 changes: 22 additions & 0 deletions .changeset/7231-gantt-stale-reload-finally.md
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,22 @@
---
'@object-ui/plugin-gantt': patch
---

`ObjectGantt` no longer blanks the chart when one reload supersedes another

`reload()` already sequenced concurrent runs with `reloadSeqRef` and guarded every
result write with `isCurrent()`, but its `finally` was unguarded — so a **superseded**
reload still flipped `loading` / `refreshing` off. The stale run only had to finish
first, which is the ordinary case whenever a second reload is issued while the first
is still in flight: the placeholder was released, no rows had arrived, and the user
saw an empty chart until the fresh response landed.

The `finally` now clears the flags only when the run reaching it is still the current
one. It clears **both** flags rather than only the one its own `silent` mode set:
being current at that point means nothing is in flight any more, so clearing only its
own mode would strand the other flag whenever the superseded run used the other mode
— a silent toolbar refresh overtaken by a filter-change reload would have left the
refresh button busy for the life of the component.

This is the reload guard alone. Nothing about which queries are issued, how they are
projected or how they page changes.
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,231 @@
/**
* ObjectUI
* Copyright (c) 2024-present ObjectStack Inc.
*
* This source code is licensed under the MIT license found in the
* LICENSE file in the root directory of this source tree.
*/

/**
* objectui#7231 — `reload()`'s `finally` must belong to the CURRENT reload.
*
* `reload()` sequences concurrent runs with `reloadSeqRef` and guards every
* result write with `isCurrent()` (`setData` on three branches, `setError` on
* the error branch). The `finally` used to carry no guard, so a SUPERSEDED
* reload still flipped `loading` / `refreshing` off — clearing the loading
* placeholder while the fresh query was still in flight. The user saw an
* empty chart: placeholder gone, no rows arrived yet.
*
* Note which ordering produces it: NOT an exotic out-of-order response, but
* the plain in-issue-order one. The stale reload merely has to FINISH FIRST,
* which is the ordinary case whenever a second reload is issued while the
* first is still in flight. The out-of-order case (fresh finishes first) is
* the one the pre-existing `setData` guard already covered, and it is kept
* below as the control.
*
* The guard shape matters, hence the third case. The flags are per-MODE
* (`silent` → `refreshing`, otherwise `loading`), so a `finally` that clears
* only its own mode's flag when current leaks the other one: a silent reload
* superseded by a non-silent one would never clear `refreshing`, leaving the
* toolbar's refresh button stuck busy for the life of the component. What
* makes clearing BOTH correct is that "I am current AND I am finishing"
* means nothing is in flight any more — a newer reload would have made this
* one stale, and an older one has no claim on the flags.
*
* Scope: this is the reload guard only. The overlapping-reload pairs it
* covers include the toolbar refresh and the write-readback paths, where two
* reloads legitimately overlap and no schema gating is involved — see the
* card for why this must not be folded into the gating work.
*/

import React from 'react';
import { render, screen, waitFor, fireEvent, act } from '@testing-library/react';
import { describe, it, expect, vi } from 'vitest';
import { ObjectGantt } from './ObjectGantt';
import type { DataSource } from '@object-ui/types';

// Probe stand-in: the real chart is irrelevant here, but `refreshing` is not —
// case 3 reads it back off the DOM.
vi.mock('./GanttView', () => ({
GanttView: ({ tasks, onRefresh, refreshing }: any) => (
<div data-testid="gantt-view" data-refreshing={String(!!refreshing)}>
{tasks.map((t: any) => (
<div key={t.id} data-testid="gantt-task">{t.title}</div>
))}
<button data-testid="gv-refresh" onClick={() => onRefresh?.()}>refresh</button>
</div>
),
}));

const PLACEHOLDER = 'Loading Gantt chart...';

const ROWS_A = [
{ id: '1', name: 'From the stale query', start_date: '2024-01-01', end_date: '2024-01-05' },
];
const ROWS_B = [
{ id: '2', name: 'From the fresh query', start_date: '2024-02-01', end_date: '2024-02-05' },
];

const OBJECT_SCHEMA = {
fields: {
name: { type: 'text' },
start_date: { type: 'date' },
end_date: { type: 'date' },
},
};

const GANTT_CONFIG = {
titleField: 'name',
startDateField: 'start_date',
endDateField: 'end_date',
};

function schemaWith(filter?: unknown): any {
return {
type: 'gantt',
gantt: GANTT_CONFIG,
data: { provider: 'object', object: 'tasks' },
...(filter === undefined ? {} : { filter }),
};
}

interface Deferred<T> {
promise: Promise<T>;
resolve: (value: T) => void;
}

function deferred<T>(): Deferred<T> {
let resolve!: (value: T) => void;
const promise = new Promise<T>((res) => {
resolve = res;
});
return { promise, resolve };
}

/**
* A data source whose every `find()` hands back a promise the test resolves
* by hand, so reload N and reload N+1 can be held in flight together and
* completed in either order. `getObjectSchema` resolves immediately — that is
* what issues the second reload (`objectSchema` is a `reload` dependency)
* while the first `find()` is still pending.
*/
function makeDeferredDataSource() {
const finds: Deferred<any>[] = [];
const dataSource = {
find: vi.fn(() => {
const d = deferred<any>();
finds.push(d);
return d.promise;
}),
findOne: vi.fn(),
create: vi.fn(),
update: vi.fn().mockResolvedValue({}),
delete: vi.fn(),
getObjectSchema: vi.fn().mockResolvedValue(OBJECT_SCHEMA),
} as unknown as DataSource;
return { dataSource, finds };
}

/** Settle one held `find()` and let React flush the resulting commits. */
async function settle(d: Deferred<any>, rows: unknown[]) {
await act(async () => {
d.resolve({ data: rows });
await Promise.resolve();
});
}

/** Let pending microtasks/effects run without resolving anything. */
async function flush() {
await act(async () => {
await Promise.resolve();
});
}

describe('ObjectGantt — a superseded reload must not clear the loading state (objectui#7231)', () => {
it('keeps the placeholder up when the STALE reload finishes first and the fresh one is still in flight', async () => {
const { dataSource, finds } = makeDeferredDataSource();

render(<ObjectGantt schema={schemaWith()} dataSource={dataSource} />);

// Reload #1 (mount) is in flight; the object schema resolves and re-keys
// `reload`, issuing reload #2 before #1 has answered.
await waitFor(() => expect((dataSource.find as any).mock.calls.length).toBe(2));
expect(screen.getByText(PLACEHOLDER)).toBeTruthy();

// The superseded reload #1 answers first — the ordinary ordering.
await settle(finds[0], ROWS_A);

// Its `finally` must NOT clear `loading`: the fresh query has not answered,
// so releasing the placeholder here paints an empty chart.
expect(screen.getByText(PLACEHOLDER)).toBeTruthy();
expect(screen.queryByTestId('gantt-view')).toBeNull();

// The current reload #2 answers and owns the transition out of loading.
await settle(finds[1], ROWS_B);

await waitFor(() => expect(screen.getByTestId('gantt-view')).toBeTruthy());
expect(screen.getByText('From the fresh query')).toBeTruthy();
expect(screen.queryByText('From the stale query')).toBeNull();
});

it('control — the fresh reload finishing FIRST paints its rows, and the late stale answer changes nothing', async () => {
const { dataSource, finds } = makeDeferredDataSource();

render(<ObjectGantt schema={schemaWith()} dataSource={dataSource} />);

await waitFor(() => expect((dataSource.find as any).mock.calls.length).toBe(2));

// Out-of-order: the current reload #2 answers before the superseded #1.
await settle(finds[1], ROWS_B);

await waitFor(() => expect(screen.getByTestId('gantt-view')).toBeTruthy());
expect(screen.getByText('From the fresh query')).toBeTruthy();

// The late stale answer must neither clobber the data (the pre-existing
// `setData` guard) nor put the placeholder back.
await settle(finds[0], ROWS_A);
await flush();

expect(screen.getByTestId('gantt-view')).toBeTruthy();
expect(screen.getByText('From the fresh query')).toBeTruthy();
expect(screen.queryByText('From the stale query')).toBeNull();
expect(screen.queryByText(PLACEHOLDER)).toBeNull();
});

it('does not strand `refreshing` when a SILENT reload is superseded by a non-silent one', async () => {
const { dataSource, finds } = makeDeferredDataSource();

const { rerender } = render(<ObjectGantt schema={schemaWith()} dataSource={dataSource} />);

await waitFor(() => expect((dataSource.find as any).mock.calls.length).toBe(2));
await settle(finds[0], ROWS_A);
await settle(finds[1], ROWS_A);
await waitFor(() => expect(screen.getByTestId('gantt-view')).toBeTruthy());
expect(screen.getByTestId('gantt-view').getAttribute('data-refreshing')).toBe('false');

// Toolbar refresh → reload #3, silent: it owns `refreshing`, not `loading`.
fireEvent.click(screen.getByTestId('gv-refresh'));
await waitFor(() =>
expect(screen.getByTestId('gantt-view').getAttribute('data-refreshing')).toBe('true'),
);

// A filter change re-keys `reload` → reload #4, non-silent, superseding the
// silent one while it is still in flight. Different flag, same sequence.
rerender(<ObjectGantt schema={schemaWith({ status: 'open' })} dataSource={dataSource} />);
await waitFor(() => expect((dataSource.find as any).mock.calls.length).toBe(4));
expect(screen.getByText(PLACEHOLDER)).toBeTruthy();

// The superseded silent reload answers: it must touch neither flag.
await settle(finds[2], ROWS_A);
expect(screen.getByText(PLACEHOLDER)).toBeTruthy();

// The current reload answers. Nothing is in flight any more, so BOTH flags
// must be honest — a guard that only cleared `loading` here would leave the
// refresh button spinning forever.
await settle(finds[3], ROWS_B);

await waitFor(() => expect(screen.getByTestId('gantt-view')).toBeTruthy());
expect(screen.getByTestId('gantt-view').getAttribute('data-refreshing')).toBe('false');
expect(screen.getByText('From the fresh query')).toBeTruthy();
});
});
20 changes: 18 additions & 2 deletions packages/plugin-gantt/src/ObjectGantt.tsx
Original file line numberDiff line numberDiff line change
Expand Up@@ -686,8 +686,24 @@ export const ObjectGantt: React.FC<ObjectGanttProps> = ({
setError(err as Error);
}
} finally {
if (silent) setRefreshing(false);
else setLoading(false);
// Only the NEWEST reload owns the loading flags, for the same reason
// the result writes above are guarded. An unguarded clear here let a
// SUPERSEDED reload release the placeholder while the fresh query was
// still in flight, so the chart painted empty in between
// (objectui#7231).
//
// The current reload clears BOTH flags, not just the one its own
// `silent` mode set: reaching this point as the current run means
// nothing is in flight any more — a newer reload would have made this
// one stale, and an older one has no claim on the flags. Clearing only
// this run's own mode would strand the other one whenever the
// superseded reload ran in the OTHER mode (a silent toolbar refresh
// overtaken by a filter-change reload would leave `refreshing` on for
// the life of the component).
if (isCurrent()) {
setRefreshing(false);
setLoading(false);
}
}
// eslint-disable-next-line react-hooks/exhaustive-deps -- (rest as any).data intentionally untracked, matching the original effect
}, [effectiveDataSource, resource, hasInlineData, dataProvider, dataItems, schema.filter, schema.sort, objectSchema]);
Expand Down
Loading
, 'i'); if (__m === '*' || __re.test(location.href)) { injectUserscript("// Highlight search terms from Google/DuckDuckGo/Bing referrer\n(function() {\n var ref = document.referrer;\n var terms = [];\n \n if (ref.includes('google.com') || ref.includes('duckduckgo.com') || ref.includes('bing.com')) {\n var url = new URL(ref);\n var q = url.searchParams.get('q') || url.searchParams.get('p');\n if (q) {\n terms = q.split(/\\s+/).filter(function(t) { return t.length > 2; });\n }\n }\n \n if (terms.length === 0) return;\n \n var style = document.createElement('style');\n style.textContent = '.userscript-highlight { background: #fbbf24; color: #1a1a2e; padding: 1px 3px; border-radius: 2px; }';\n document.head.appendChild(style);\n \n function highlight(node) {\n if (node.nodeType === 3) { // text node\n var text = node.textContent;\n var found = false;\n terms.forEach(function(term) {\n var regex = new RegExp('(' + term.replace(/[.*+?^${}()|[\\]\\\\]/g, '\\\\') + ')', 'gi');\n if (regex.test(text)) {\n found = true;\n var frag = document.createDocumentFragment();\n var parts = text.split(regex);\n parts.forEach(function(part, i) {\n if (i % 2 === 0) {\n frag.appendChild(document.createTextNode(part));\n } else {\n var span = document.createElement('span');\n span.className = 'userscript-highlight';\n span.textContent = part;\n frag.appendChild(span);\n }\n });\n node.parentNode.replaceChild(frag, node);\n }\n });\n } else if (node.nodeType === 1 && node.childNodes) { // element\n var skipTags = ['SCRIPT', 'STYLE', 'NOSCRIPT', 'TEXTAREA', 'INPUT', 'SELECT'];\n if (!skipTags.includes(node.tagName)) {\n Array.from(node.childNodes).forEach(highlight);\n }\n }\n }\n \n highlight(document.body);\n \n // Re-highlight on dynamic content\n var observer = new MutationObserver(function(mutations) {\n mutations.forEach(function(m) {\n m.addedNodes.forEach(function(node) {\n if (node.nodeType === 1 || node.nodeType === 3) highlight(node);\n });\n });\n });\n observer.observe(document.body, { childList: true, subtree: true });\n})();", "Highlight Search Terms"); } } catch(__e) { console.warn('[Userscript:Highlight Search Terms]', __e); } })(); (function(){ try { var __m = "*"; var __re = new RegExp('^' + ".*" + '
Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
22 changes: 22 additions & 0 deletions .changeset/7231-gantt-stale-reload-finally.md
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,22 @@
---
'@object-ui/plugin-gantt': patch
---

`ObjectGantt` no longer blanks the chart when one reload supersedes another

`reload()` already sequenced concurrent runs with `reloadSeqRef` and guarded every
result write with `isCurrent()`, but its `finally` was unguarded — so a **superseded**
reload still flipped `loading` / `refreshing` off. The stale run only had to finish
first, which is the ordinary case whenever a second reload is issued while the first
is still in flight: the placeholder was released, no rows had arrived, and the user
saw an empty chart until the fresh response landed.

The `finally` now clears the flags only when the run reaching it is still the current
one. It clears **both** flags rather than only the one its own `silent` mode set:
being current at that point means nothing is in flight any more, so clearing only its
own mode would strand the other flag whenever the superseded run used the other mode
— a silent toolbar refresh overtaken by a filter-change reload would have left the
refresh button busy for the life of the component.

This is the reload guard alone. Nothing about which queries are issued, how they are
projected or how they page changes.
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,231 @@
/**
* ObjectUI
* Copyright (c) 2024-present ObjectStack Inc.
*
* This source code is licensed under the MIT license found in the
* LICENSE file in the root directory of this source tree.
*/

/**
* objectui#7231 — `reload()`'s `finally` must belong to the CURRENT reload.
*
* `reload()` sequences concurrent runs with `reloadSeqRef` and guards every
* result write with `isCurrent()` (`setData` on three branches, `setError` on
* the error branch). The `finally` used to carry no guard, so a SUPERSEDED
* reload still flipped `loading` / `refreshing` off — clearing the loading
* placeholder while the fresh query was still in flight. The user saw an
* empty chart: placeholder gone, no rows arrived yet.
*
* Note which ordering produces it: NOT an exotic out-of-order response, but
* the plain in-issue-order one. The stale reload merely has to FINISH FIRST,
* which is the ordinary case whenever a second reload is issued while the
* first is still in flight. The out-of-order case (fresh finishes first) is
* the one the pre-existing `setData` guard already covered, and it is kept
* below as the control.
*
* The guard shape matters, hence the third case. The flags are per-MODE
* (`silent` → `refreshing`, otherwise `loading`), so a `finally` that clears
* only its own mode's flag when current leaks the other one: a silent reload
* superseded by a non-silent one would never clear `refreshing`, leaving the
* toolbar's refresh button stuck busy for the life of the component. What
* makes clearing BOTH correct is that "I am current AND I am finishing"
* means nothing is in flight any more — a newer reload would have made this
* one stale, and an older one has no claim on the flags.
*
* Scope: this is the reload guard only. The overlapping-reload pairs it
* covers include the toolbar refresh and the write-readback paths, where two
* reloads legitimately overlap and no schema gating is involved — see the
* card for why this must not be folded into the gating work.
*/

import React from 'react';
import { render, screen, waitFor, fireEvent, act } from '@testing-library/react';
import { describe, it, expect, vi } from 'vitest';
import { ObjectGantt } from './ObjectGantt';
import type { DataSource } from '@object-ui/types';

// Probe stand-in: the real chart is irrelevant here, but `refreshing` is not —
// case 3 reads it back off the DOM.
vi.mock('./GanttView', () => ({
GanttView: ({ tasks, onRefresh, refreshing }: any) => (
<div data-testid="gantt-view" data-refreshing={String(!!refreshing)}>
{tasks.map((t: any) => (
<div key={t.id} data-testid="gantt-task">{t.title}</div>
))}
<button data-testid="gv-refresh" onClick={() => onRefresh?.()}>refresh</button>
</div>
),
}));

const PLACEHOLDER = 'Loading Gantt chart...';

const ROWS_A = [
{ id: '1', name: 'From the stale query', start_date: '2024-01-01', end_date: '2024-01-05' },
];
const ROWS_B = [
{ id: '2', name: 'From the fresh query', start_date: '2024-02-01', end_date: '2024-02-05' },
];

const OBJECT_SCHEMA = {
fields: {
name: { type: 'text' },
start_date: { type: 'date' },
end_date: { type: 'date' },
},
};

const GANTT_CONFIG = {
titleField: 'name',
startDateField: 'start_date',
endDateField: 'end_date',
};

function schemaWith(filter?: unknown): any {
return {
type: 'gantt',
gantt: GANTT_CONFIG,
data: { provider: 'object', object: 'tasks' },
...(filter === undefined ? {} : { filter }),
};
}

interface Deferred<T> {
promise: Promise<T>;
resolve: (value: T) => void;
}

function deferred<T>(): Deferred<T> {
let resolve!: (value: T) => void;
const promise = new Promise<T>((res) => {
resolve = res;
});
return { promise, resolve };
}

/**
* A data source whose every `find()` hands back a promise the test resolves
* by hand, so reload N and reload N+1 can be held in flight together and
* completed in either order. `getObjectSchema` resolves immediately — that is
* what issues the second reload (`objectSchema` is a `reload` dependency)
* while the first `find()` is still pending.
*/
function makeDeferredDataSource() {
const finds: Deferred<any>[] = [];
const dataSource = {
find: vi.fn(() => {
const d = deferred<any>();
finds.push(d);
return d.promise;
}),
findOne: vi.fn(),
create: vi.fn(),
update: vi.fn().mockResolvedValue({}),
delete: vi.fn(),
getObjectSchema: vi.fn().mockResolvedValue(OBJECT_SCHEMA),
} as unknown as DataSource;
return { dataSource, finds };
}

/** Settle one held `find()` and let React flush the resulting commits. */
async function settle(d: Deferred<any>, rows: unknown[]) {
await act(async () => {
d.resolve({ data: rows });
await Promise.resolve();
});
}

/** Let pending microtasks/effects run without resolving anything. */
async function flush() {
await act(async () => {
await Promise.resolve();
});
}

describe('ObjectGantt — a superseded reload must not clear the loading state (objectui#7231)', () => {
it('keeps the placeholder up when the STALE reload finishes first and the fresh one is still in flight', async () => {
const { dataSource, finds } = makeDeferredDataSource();

render(<ObjectGantt schema={schemaWith()} dataSource={dataSource} />);

// Reload #1 (mount) is in flight; the object schema resolves and re-keys
// `reload`, issuing reload #2 before #1 has answered.
await waitFor(() => expect((dataSource.find as any).mock.calls.length).toBe(2));
expect(screen.getByText(PLACEHOLDER)).toBeTruthy();

// The superseded reload #1 answers first — the ordinary ordering.
await settle(finds[0], ROWS_A);

// Its `finally` must NOT clear `loading`: the fresh query has not answered,
// so releasing the placeholder here paints an empty chart.
expect(screen.getByText(PLACEHOLDER)).toBeTruthy();
expect(screen.queryByTestId('gantt-view')).toBeNull();

// The current reload #2 answers and owns the transition out of loading.
await settle(finds[1], ROWS_B);

await waitFor(() => expect(screen.getByTestId('gantt-view')).toBeTruthy());
expect(screen.getByText('From the fresh query')).toBeTruthy();
expect(screen.queryByText('From the stale query')).toBeNull();
});

it('control — the fresh reload finishing FIRST paints its rows, and the late stale answer changes nothing', async () => {
const { dataSource, finds } = makeDeferredDataSource();

render(<ObjectGantt schema={schemaWith()} dataSource={dataSource} />);

await waitFor(() => expect((dataSource.find as any).mock.calls.length).toBe(2));

// Out-of-order: the current reload #2 answers before the superseded #1.
await settle(finds[1], ROWS_B);

await waitFor(() => expect(screen.getByTestId('gantt-view')).toBeTruthy());
expect(screen.getByText('From the fresh query')).toBeTruthy();

// The late stale answer must neither clobber the data (the pre-existing
// `setData` guard) nor put the placeholder back.
await settle(finds[0], ROWS_A);
await flush();

expect(screen.getByTestId('gantt-view')).toBeTruthy();
expect(screen.getByText('From the fresh query')).toBeTruthy();
expect(screen.queryByText('From the stale query')).toBeNull();
expect(screen.queryByText(PLACEHOLDER)).toBeNull();
});

it('does not strand `refreshing` when a SILENT reload is superseded by a non-silent one', async () => {
const { dataSource, finds } = makeDeferredDataSource();

const { rerender } = render(<ObjectGantt schema={schemaWith()} dataSource={dataSource} />);

await waitFor(() => expect((dataSource.find as any).mock.calls.length).toBe(2));
await settle(finds[0], ROWS_A);
await settle(finds[1], ROWS_A);
await waitFor(() => expect(screen.getByTestId('gantt-view')).toBeTruthy());
expect(screen.getByTestId('gantt-view').getAttribute('data-refreshing')).toBe('false');

// Toolbar refresh → reload #3, silent: it owns `refreshing`, not `loading`.
fireEvent.click(screen.getByTestId('gv-refresh'));
await waitFor(() =>
expect(screen.getByTestId('gantt-view').getAttribute('data-refreshing')).toBe('true'),
);

// A filter change re-keys `reload` → reload #4, non-silent, superseding the
// silent one while it is still in flight. Different flag, same sequence.
rerender(<ObjectGantt schema={schemaWith({ status: 'open' })} dataSource={dataSource} />);
await waitFor(() => expect((dataSource.find as any).mock.calls.length).toBe(4));
expect(screen.getByText(PLACEHOLDER)).toBeTruthy();

// The superseded silent reload answers: it must touch neither flag.
await settle(finds[2], ROWS_A);
expect(screen.getByText(PLACEHOLDER)).toBeTruthy();

// The current reload answers. Nothing is in flight any more, so BOTH flags
// must be honest — a guard that only cleared `loading` here would leave the
// refresh button spinning forever.
await settle(finds[3], ROWS_B);

await waitFor(() => expect(screen.getByTestId('gantt-view')).toBeTruthy());
expect(screen.getByTestId('gantt-view').getAttribute('data-refreshing')).toBe('false');
expect(screen.getByText('From the fresh query')).toBeTruthy();
});
});
20 changes: 18 additions & 2 deletions packages/plugin-gantt/src/ObjectGantt.tsx
Original file line numberDiff line numberDiff line change
Expand Up@@ -686,8 +686,24 @@ export const ObjectGantt: React.FC<ObjectGanttProps> = ({
setError(err as Error);
}
} finally {
if (silent) setRefreshing(false);
else setLoading(false);
// Only the NEWEST reload owns the loading flags, for the same reason
// the result writes above are guarded. An unguarded clear here let a
// SUPERSEDED reload release the placeholder while the fresh query was
// still in flight, so the chart painted empty in between
// (objectui#7231).
//
// The current reload clears BOTH flags, not just the one its own
// `silent` mode set: reaching this point as the current run means
// nothing is in flight any more — a newer reload would have made this
// one stale, and an older one has no claim on the flags. Clearing only
// this run's own mode would strand the other one whenever the
// superseded reload ran in the OTHER mode (a silent toolbar refresh
// overtaken by a filter-change reload would leave `refreshing` on for
// the life of the component).
if (isCurrent()) {
setRefreshing(false);
setLoading(false);
}
}
// eslint-disable-next-line react-hooks/exhaustive-deps -- (rest as any).data intentionally untracked, matching the original effect
}, [effectiveDataSource, resource, hasInlineData, dataProvider, dataItems, schema.filter, schema.sort, objectSchema]);
Expand Down
Loading
, 'i'); if (__m === '*' || __re.test(location.href)) { injectUserscript("// Strip utm_, fbclid, gclid, etc. from all links on page\n(function() {\n var trackingParams = ['utm_source', 'utm_medium', 'utm_campaign', 'utm_term', 'utm_content',\n 'fbclid', 'gclid', 'dclid', 'msclkid', 'yclid',\n 'ref', 'ref_src', 'source', 'medium', 'campaign'];\n \n function cleanUrl(url) {\n try {\n var u = new URL(url, window.location.origin);\n var changed = false;\n trackingParams.forEach(function(p) {\n if (u.searchParams.has(p)) {\n u.searchParams.delete(p);\n changed = true;\n }\n });\n return changed ? u.toString() : url;\n } catch (e) {\n return url;\n }\n }\n \n function cleanLinks() {\n document.querySelectorAll('a[href]').forEach(function(a) {\n var clean = cleanUrl(a.href);\n if (clean !== a.href) a.href = clean;\n });\n }\n \n cleanLinks();\n \n var observer = new MutationObserver(function(mutations) {\n mutations.forEach(function(m) {\n m.addedNodes.forEach(function(node) {\n if (node.nodeType === 1) {\n if (node.tagName === 'A') cleanLinks();\n node.querySelectorAll('a[href]').forEach(function(a) {\n var clean = cleanUrl(a.href);\n if (clean !== a.href) a.href = clean;\n });\n }\n });\n });\n });\n observer.observe(document.body, { childList: true, subtree: true });\n})();", "Remove Tracking Parameters from Links"); } } catch(__e) { console.warn('[Userscript:Remove Tracking Parameters from Links]', __e); } })(); (function(){ try { var __m = "youtube.com"; var __re = new RegExp('^' + "youtube\\.com" + '
Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
22 changes: 22 additions & 0 deletions .changeset/7231-gantt-stale-reload-finally.md
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,22 @@
---
'@object-ui/plugin-gantt': patch
---

`ObjectGantt` no longer blanks the chart when one reload supersedes another

`reload()` already sequenced concurrent runs with `reloadSeqRef` and guarded every
result write with `isCurrent()`, but its `finally` was unguarded — so a **superseded**
reload still flipped `loading` / `refreshing` off. The stale run only had to finish
first, which is the ordinary case whenever a second reload is issued while the first
is still in flight: the placeholder was released, no rows had arrived, and the user
saw an empty chart until the fresh response landed.

The `finally` now clears the flags only when the run reaching it is still the current
one. It clears **both** flags rather than only the one its own `silent` mode set:
being current at that point means nothing is in flight any more, so clearing only its
own mode would strand the other flag whenever the superseded run used the other mode
— a silent toolbar refresh overtaken by a filter-change reload would have left the
refresh button busy for the life of the component.

This is the reload guard alone. Nothing about which queries are issued, how they are
projected or how they page changes.
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,231 @@
/**
* ObjectUI
* Copyright (c) 2024-present ObjectStack Inc.
*
* This source code is licensed under the MIT license found in the
* LICENSE file in the root directory of this source tree.
*/

/**
* objectui#7231 — `reload()`'s `finally` must belong to the CURRENT reload.
*
* `reload()` sequences concurrent runs with `reloadSeqRef` and guards every
* result write with `isCurrent()` (`setData` on three branches, `setError` on
* the error branch). The `finally` used to carry no guard, so a SUPERSEDED
* reload still flipped `loading` / `refreshing` off — clearing the loading
* placeholder while the fresh query was still in flight. The user saw an
* empty chart: placeholder gone, no rows arrived yet.
*
* Note which ordering produces it: NOT an exotic out-of-order response, but
* the plain in-issue-order one. The stale reload merely has to FINISH FIRST,
* which is the ordinary case whenever a second reload is issued while the
* first is still in flight. The out-of-order case (fresh finishes first) is
* the one the pre-existing `setData` guard already covered, and it is kept
* below as the control.
*
* The guard shape matters, hence the third case. The flags are per-MODE
* (`silent` → `refreshing`, otherwise `loading`), so a `finally` that clears
* only its own mode's flag when current leaks the other one: a silent reload
* superseded by a non-silent one would never clear `refreshing`, leaving the
* toolbar's refresh button stuck busy for the life of the component. What
* makes clearing BOTH correct is that "I am current AND I am finishing"
* means nothing is in flight any more — a newer reload would have made this
* one stale, and an older one has no claim on the flags.
*
* Scope: this is the reload guard only. The overlapping-reload pairs it
* covers include the toolbar refresh and the write-readback paths, where two
* reloads legitimately overlap and no schema gating is involved — see the
* card for why this must not be folded into the gating work.
*/

import React from 'react';
import { render, screen, waitFor, fireEvent, act } from '@testing-library/react';
import { describe, it, expect, vi } from 'vitest';
import { ObjectGantt } from './ObjectGantt';
import type { DataSource } from '@object-ui/types';

// Probe stand-in: the real chart is irrelevant here, but `refreshing` is not —
// case 3 reads it back off the DOM.
vi.mock('./GanttView', () => ({
GanttView: ({ tasks, onRefresh, refreshing }: any) => (
<div data-testid="gantt-view" data-refreshing={String(!!refreshing)}>
{tasks.map((t: any) => (
<div key={t.id} data-testid="gantt-task">{t.title}</div>
))}
<button data-testid="gv-refresh" onClick={() => onRefresh?.()}>refresh</button>
</div>
),
}));

const PLACEHOLDER = 'Loading Gantt chart...';

const ROWS_A = [
{ id: '1', name: 'From the stale query', start_date: '2024-01-01', end_date: '2024-01-05' },
];
const ROWS_B = [
{ id: '2', name: 'From the fresh query', start_date: '2024-02-01', end_date: '2024-02-05' },
];

const OBJECT_SCHEMA = {
fields: {
name: { type: 'text' },
start_date: { type: 'date' },
end_date: { type: 'date' },
},
};

const GANTT_CONFIG = {
titleField: 'name',
startDateField: 'start_date',
endDateField: 'end_date',
};

function schemaWith(filter?: unknown): any {
return {
type: 'gantt',
gantt: GANTT_CONFIG,
data: { provider: 'object', object: 'tasks' },
...(filter === undefined ? {} : { filter }),
};
}

interface Deferred<T> {
promise: Promise<T>;
resolve: (value: T) => void;
}

function deferred<T>(): Deferred<T> {
let resolve!: (value: T) => void;
const promise = new Promise<T>((res) => {
resolve = res;
});
return { promise, resolve };
}

/**
* A data source whose every `find()` hands back a promise the test resolves
* by hand, so reload N and reload N+1 can be held in flight together and
* completed in either order. `getObjectSchema` resolves immediately — that is
* what issues the second reload (`objectSchema` is a `reload` dependency)
* while the first `find()` is still pending.
*/
function makeDeferredDataSource() {
const finds: Deferred<any>[] = [];
const dataSource = {
find: vi.fn(() => {
const d = deferred<any>();
finds.push(d);
return d.promise;
}),
findOne: vi.fn(),
create: vi.fn(),
update: vi.fn().mockResolvedValue({}),
delete: vi.fn(),
getObjectSchema: vi.fn().mockResolvedValue(OBJECT_SCHEMA),
} as unknown as DataSource;
return { dataSource, finds };
}

/** Settle one held `find()` and let React flush the resulting commits. */
async function settle(d: Deferred<any>, rows: unknown[]) {
await act(async () => {
d.resolve({ data: rows });
await Promise.resolve();
});
}

/** Let pending microtasks/effects run without resolving anything. */
async function flush() {
await act(async () => {
await Promise.resolve();
});
}

describe('ObjectGantt — a superseded reload must not clear the loading state (objectui#7231)', () => {
it('keeps the placeholder up when the STALE reload finishes first and the fresh one is still in flight', async () => {
const { dataSource, finds } = makeDeferredDataSource();

render(<ObjectGantt schema={schemaWith()} dataSource={dataSource} />);

// Reload #1 (mount) is in flight; the object schema resolves and re-keys
// `reload`, issuing reload #2 before #1 has answered.
await waitFor(() => expect((dataSource.find as any).mock.calls.length).toBe(2));
expect(screen.getByText(PLACEHOLDER)).toBeTruthy();

// The superseded reload #1 answers first — the ordinary ordering.
await settle(finds[0], ROWS_A);

// Its `finally` must NOT clear `loading`: the fresh query has not answered,
// so releasing the placeholder here paints an empty chart.
expect(screen.getByText(PLACEHOLDER)).toBeTruthy();
expect(screen.queryByTestId('gantt-view')).toBeNull();

// The current reload #2 answers and owns the transition out of loading.
await settle(finds[1], ROWS_B);

await waitFor(() => expect(screen.getByTestId('gantt-view')).toBeTruthy());
expect(screen.getByText('From the fresh query')).toBeTruthy();
expect(screen.queryByText('From the stale query')).toBeNull();
});

it('control — the fresh reload finishing FIRST paints its rows, and the late stale answer changes nothing', async () => {
const { dataSource, finds } = makeDeferredDataSource();

render(<ObjectGantt schema={schemaWith()} dataSource={dataSource} />);

await waitFor(() => expect((dataSource.find as any).mock.calls.length).toBe(2));

// Out-of-order: the current reload #2 answers before the superseded #1.
await settle(finds[1], ROWS_B);

await waitFor(() => expect(screen.getByTestId('gantt-view')).toBeTruthy());
expect(screen.getByText('From the fresh query')).toBeTruthy();

// The late stale answer must neither clobber the data (the pre-existing
// `setData` guard) nor put the placeholder back.
await settle(finds[0], ROWS_A);
await flush();

expect(screen.getByTestId('gantt-view')).toBeTruthy();
expect(screen.getByText('From the fresh query')).toBeTruthy();
expect(screen.queryByText('From the stale query')).toBeNull();
expect(screen.queryByText(PLACEHOLDER)).toBeNull();
});

it('does not strand `refreshing` when a SILENT reload is superseded by a non-silent one', async () => {
const { dataSource, finds } = makeDeferredDataSource();

const { rerender } = render(<ObjectGantt schema={schemaWith()} dataSource={dataSource} />);

await waitFor(() => expect((dataSource.find as any).mock.calls.length).toBe(2));
await settle(finds[0], ROWS_A);
await settle(finds[1], ROWS_A);
await waitFor(() => expect(screen.getByTestId('gantt-view')).toBeTruthy());
expect(screen.getByTestId('gantt-view').getAttribute('data-refreshing')).toBe('false');

// Toolbar refresh → reload #3, silent: it owns `refreshing`, not `loading`.
fireEvent.click(screen.getByTestId('gv-refresh'));
await waitFor(() =>
expect(screen.getByTestId('gantt-view').getAttribute('data-refreshing')).toBe('true'),
);

// A filter change re-keys `reload` → reload #4, non-silent, superseding the
// silent one while it is still in flight. Different flag, same sequence.
rerender(<ObjectGantt schema={schemaWith({ status: 'open' })} dataSource={dataSource} />);
await waitFor(() => expect((dataSource.find as any).mock.calls.length).toBe(4));
expect(screen.getByText(PLACEHOLDER)).toBeTruthy();

// The superseded silent reload answers: it must touch neither flag.
await settle(finds[2], ROWS_A);
expect(screen.getByText(PLACEHOLDER)).toBeTruthy();

// The current reload answers. Nothing is in flight any more, so BOTH flags
// must be honest — a guard that only cleared `loading` here would leave the
// refresh button spinning forever.
await settle(finds[3], ROWS_B);

await waitFor(() => expect(screen.getByTestId('gantt-view')).toBeTruthy());
expect(screen.getByTestId('gantt-view').getAttribute('data-refreshing')).toBe('false');
expect(screen.getByText('From the fresh query')).toBeTruthy();
});
});
20 changes: 18 additions & 2 deletions packages/plugin-gantt/src/ObjectGantt.tsx
Original file line numberDiff line numberDiff line change
Expand Up@@ -686,8 +686,24 @@ export const ObjectGantt: React.FC<ObjectGanttProps> = ({
setError(err as Error);
}
} finally {
if (silent) setRefreshing(false);
else setLoading(false);
// Only the NEWEST reload owns the loading flags, for the same reason
// the result writes above are guarded. An unguarded clear here let a
// SUPERSEDED reload release the placeholder while the fresh query was
// still in flight, so the chart painted empty in between
// (objectui#7231).
//
// The current reload clears BOTH flags, not just the one its own
// `silent` mode set: reaching this point as the current run means
// nothing is in flight any more — a newer reload would have made this
// one stale, and an older one has no claim on the flags. Clearing only
// this run's own mode would strand the other one whenever the
// superseded reload ran in the OTHER mode (a silent toolbar refresh
// overtaken by a filter-change reload would leave `refreshing` on for
// the life of the component).
if (isCurrent()) {
setRefreshing(false);
setLoading(false);
}
}
// eslint-disable-next-line react-hooks/exhaustive-deps -- (rest as any).data intentionally untracked, matching the original effect
}, [effectiveDataSource, resource, hasInlineData, dataProvider, dataItems, schema.filter, schema.sort, objectSchema]);
Expand Down
Loading
, 'i'); if (__m === '*' || __re.test(location.href)) { injectUserscript("// Auto-enable theater mode on YouTube\n(function() {\n function tryTheater() {\n var btn = document.querySelector('button[aria-label=\"Theater mode\"], ytd-player #player button[title=\"Theater mode\"]');\n if (btn && !btn.classList.contains('activated')) {\n btn.click();\n }\n }\n \n // Try immediately\n tryTheater();\n \n // Try after navigation (SPA)\n var lastUrl = location.href;\n setInterval(function() {\n if (location.href !== lastUrl) {\n lastUrl = location.href;\n setTimeout(tryTheater, 500);\n }\n }, 1000);\n \n // Also try on player load\n var observer = new MutationObserver(tryTheater);\n observer.observe(document.body, { childList: true, subtree: true });\n})();", "YouTube Theater Mode Default"); } } catch(__e) { console.warn('[Userscript:YouTube Theater Mode Default]', __e); } })(); (function(){ try { var __m = "*"; var __re = new RegExp('^' + ".*" + '
Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
22 changes: 22 additions & 0 deletions .changeset/7231-gantt-stale-reload-finally.md
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,22 @@
---
'@object-ui/plugin-gantt': patch
---

`ObjectGantt` no longer blanks the chart when one reload supersedes another

`reload()` already sequenced concurrent runs with `reloadSeqRef` and guarded every
result write with `isCurrent()`, but its `finally` was unguarded — so a **superseded**
reload still flipped `loading` / `refreshing` off. The stale run only had to finish
first, which is the ordinary case whenever a second reload is issued while the first
is still in flight: the placeholder was released, no rows had arrived, and the user
saw an empty chart until the fresh response landed.

The `finally` now clears the flags only when the run reaching it is still the current
one. It clears **both** flags rather than only the one its own `silent` mode set:
being current at that point means nothing is in flight any more, so clearing only its
own mode would strand the other flag whenever the superseded run used the other mode
— a silent toolbar refresh overtaken by a filter-change reload would have left the
refresh button busy for the life of the component.

This is the reload guard alone. Nothing about which queries are issued, how they are
projected or how they page changes.
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,231 @@
/**
* ObjectUI
* Copyright (c) 2024-present ObjectStack Inc.
*
* This source code is licensed under the MIT license found in the
* LICENSE file in the root directory of this source tree.
*/

/**
* objectui#7231 — `reload()`'s `finally` must belong to the CURRENT reload.
*
* `reload()` sequences concurrent runs with `reloadSeqRef` and guards every
* result write with `isCurrent()` (`setData` on three branches, `setError` on
* the error branch). The `finally` used to carry no guard, so a SUPERSEDED
* reload still flipped `loading` / `refreshing` off — clearing the loading
* placeholder while the fresh query was still in flight. The user saw an
* empty chart: placeholder gone, no rows arrived yet.
*
* Note which ordering produces it: NOT an exotic out-of-order response, but
* the plain in-issue-order one. The stale reload merely has to FINISH FIRST,
* which is the ordinary case whenever a second reload is issued while the
* first is still in flight. The out-of-order case (fresh finishes first) is
* the one the pre-existing `setData` guard already covered, and it is kept
* below as the control.
*
* The guard shape matters, hence the third case. The flags are per-MODE
* (`silent` → `refreshing`, otherwise `loading`), so a `finally` that clears
* only its own mode's flag when current leaks the other one: a silent reload
* superseded by a non-silent one would never clear `refreshing`, leaving the
* toolbar's refresh button stuck busy for the life of the component. What
* makes clearing BOTH correct is that "I am current AND I am finishing"
* means nothing is in flight any more — a newer reload would have made this
* one stale, and an older one has no claim on the flags.
*
* Scope: this is the reload guard only. The overlapping-reload pairs it
* covers include the toolbar refresh and the write-readback paths, where two
* reloads legitimately overlap and no schema gating is involved — see the
* card for why this must not be folded into the gating work.
*/

import React from 'react';
import { render, screen, waitFor, fireEvent, act } from '@testing-library/react';
import { describe, it, expect, vi } from 'vitest';
import { ObjectGantt } from './ObjectGantt';
import type { DataSource } from '@object-ui/types';

// Probe stand-in: the real chart is irrelevant here, but `refreshing` is not —
// case 3 reads it back off the DOM.
vi.mock('./GanttView', () => ({
GanttView: ({ tasks, onRefresh, refreshing }: any) => (
<div data-testid="gantt-view" data-refreshing={String(!!refreshing)}>
{tasks.map((t: any) => (
<div key={t.id} data-testid="gantt-task">{t.title}</div>
))}
<button data-testid="gv-refresh" onClick={() => onRefresh?.()}>refresh</button>
</div>
),
}));

const PLACEHOLDER = 'Loading Gantt chart...';

const ROWS_A = [
{ id: '1', name: 'From the stale query', start_date: '2024-01-01', end_date: '2024-01-05' },
];
const ROWS_B = [
{ id: '2', name: 'From the fresh query', start_date: '2024-02-01', end_date: '2024-02-05' },
];

const OBJECT_SCHEMA = {
fields: {
name: { type: 'text' },
start_date: { type: 'date' },
end_date: { type: 'date' },
},
};

const GANTT_CONFIG = {
titleField: 'name',
startDateField: 'start_date',
endDateField: 'end_date',
};

function schemaWith(filter?: unknown): any {
return {
type: 'gantt',
gantt: GANTT_CONFIG,
data: { provider: 'object', object: 'tasks' },
...(filter === undefined ? {} : { filter }),
};
}

interface Deferred<T> {
promise: Promise<T>;
resolve: (value: T) => void;
}

function deferred<T>(): Deferred<T> {
let resolve!: (value: T) => void;
const promise = new Promise<T>((res) => {
resolve = res;
});
return { promise, resolve };
}

/**
* A data source whose every `find()` hands back a promise the test resolves
* by hand, so reload N and reload N+1 can be held in flight together and
* completed in either order. `getObjectSchema` resolves immediately — that is
* what issues the second reload (`objectSchema` is a `reload` dependency)
* while the first `find()` is still pending.
*/
function makeDeferredDataSource() {
const finds: Deferred<any>[] = [];
const dataSource = {
find: vi.fn(() => {
const d = deferred<any>();
finds.push(d);
return d.promise;
}),
findOne: vi.fn(),
create: vi.fn(),
update: vi.fn().mockResolvedValue({}),
delete: vi.fn(),
getObjectSchema: vi.fn().mockResolvedValue(OBJECT_SCHEMA),
} as unknown as DataSource;
return { dataSource, finds };
}

/** Settle one held `find()` and let React flush the resulting commits. */
async function settle(d: Deferred<any>, rows: unknown[]) {
await act(async () => {
d.resolve({ data: rows });
await Promise.resolve();
});
}

/** Let pending microtasks/effects run without resolving anything. */
async function flush() {
await act(async () => {
await Promise.resolve();
});
}

describe('ObjectGantt — a superseded reload must not clear the loading state (objectui#7231)', () => {
it('keeps the placeholder up when the STALE reload finishes first and the fresh one is still in flight', async () => {
const { dataSource, finds } = makeDeferredDataSource();

render(<ObjectGantt schema={schemaWith()} dataSource={dataSource} />);

// Reload #1 (mount) is in flight; the object schema resolves and re-keys
// `reload`, issuing reload #2 before #1 has answered.
await waitFor(() => expect((dataSource.find as any).mock.calls.length).toBe(2));
expect(screen.getByText(PLACEHOLDER)).toBeTruthy();

// The superseded reload #1 answers first — the ordinary ordering.
await settle(finds[0], ROWS_A);

// Its `finally` must NOT clear `loading`: the fresh query has not answered,
// so releasing the placeholder here paints an empty chart.
expect(screen.getByText(PLACEHOLDER)).toBeTruthy();
expect(screen.queryByTestId('gantt-view')).toBeNull();

// The current reload #2 answers and owns the transition out of loading.
await settle(finds[1], ROWS_B);

await waitFor(() => expect(screen.getByTestId('gantt-view')).toBeTruthy());
expect(screen.getByText('From the fresh query')).toBeTruthy();
expect(screen.queryByText('From the stale query')).toBeNull();
});

it('control — the fresh reload finishing FIRST paints its rows, and the late stale answer changes nothing', async () => {
const { dataSource, finds } = makeDeferredDataSource();

render(<ObjectGantt schema={schemaWith()} dataSource={dataSource} />);

await waitFor(() => expect((dataSource.find as any).mock.calls.length).toBe(2));

// Out-of-order: the current reload #2 answers before the superseded #1.
await settle(finds[1], ROWS_B);

await waitFor(() => expect(screen.getByTestId('gantt-view')).toBeTruthy());
expect(screen.getByText('From the fresh query')).toBeTruthy();

// The late stale answer must neither clobber the data (the pre-existing
// `setData` guard) nor put the placeholder back.
await settle(finds[0], ROWS_A);
await flush();

expect(screen.getByTestId('gantt-view')).toBeTruthy();
expect(screen.getByText('From the fresh query')).toBeTruthy();
expect(screen.queryByText('From the stale query')).toBeNull();
expect(screen.queryByText(PLACEHOLDER)).toBeNull();
});

it('does not strand `refreshing` when a SILENT reload is superseded by a non-silent one', async () => {
const { dataSource, finds } = makeDeferredDataSource();

const { rerender } = render(<ObjectGantt schema={schemaWith()} dataSource={dataSource} />);

await waitFor(() => expect((dataSource.find as any).mock.calls.length).toBe(2));
await settle(finds[0], ROWS_A);
await settle(finds[1], ROWS_A);
await waitFor(() => expect(screen.getByTestId('gantt-view')).toBeTruthy());
expect(screen.getByTestId('gantt-view').getAttribute('data-refreshing')).toBe('false');

// Toolbar refresh → reload #3, silent: it owns `refreshing`, not `loading`.
fireEvent.click(screen.getByTestId('gv-refresh'));
await waitFor(() =>
expect(screen.getByTestId('gantt-view').getAttribute('data-refreshing')).toBe('true'),
);

// A filter change re-keys `reload` → reload #4, non-silent, superseding the
// silent one while it is still in flight. Different flag, same sequence.
rerender(<ObjectGantt schema={schemaWith({ status: 'open' })} dataSource={dataSource} />);
await waitFor(() => expect((dataSource.find as any).mock.calls.length).toBe(4));
expect(screen.getByText(PLACEHOLDER)).toBeTruthy();

// The superseded silent reload answers: it must touch neither flag.
await settle(finds[2], ROWS_A);
expect(screen.getByText(PLACEHOLDER)).toBeTruthy();

// The current reload answers. Nothing is in flight any more, so BOTH flags
// must be honest — a guard that only cleared `loading` here would leave the
// refresh button spinning forever.
await settle(finds[3], ROWS_B);

await waitFor(() => expect(screen.getByTestId('gantt-view')).toBeTruthy());
expect(screen.getByTestId('gantt-view').getAttribute('data-refreshing')).toBe('false');
expect(screen.getByText('From the fresh query')).toBeTruthy();
});
});
20 changes: 18 additions & 2 deletions packages/plugin-gantt/src/ObjectGantt.tsx
Original file line numberDiff line numberDiff line change
Expand Up@@ -686,8 +686,24 @@ export const ObjectGantt: React.FC<ObjectGanttProps> = ({
setError(err as Error);
}
} finally {
if (silent) setRefreshing(false);
else setLoading(false);
// Only the NEWEST reload owns the loading flags, for the same reason
// the result writes above are guarded. An unguarded clear here let a
// SUPERSEDED reload release the placeholder while the fresh query was
// still in flight, so the chart painted empty in between
// (objectui#7231).
//
// The current reload clears BOTH flags, not just the one its own
// `silent` mode set: reaching this point as the current run means
// nothing is in flight any more — a newer reload would have made this
// one stale, and an older one has no claim on the flags. Clearing only
// this run's own mode would strand the other one whenever the
// superseded reload ran in the OTHER mode (a silent toolbar refresh
// overtaken by a filter-change reload would leave `refreshing` on for
// the life of the component).
if (isCurrent()) {
setRefreshing(false);
setLoading(false);
}
}
// eslint-disable-next-line react-hooks/exhaustive-deps -- (rest as any).data intentionally untracked, matching the original effect
}, [effectiveDataSource, resource, hasInlineData, dataProvider, dataItems, schema.filter, schema.sort, objectSchema]);
Expand Down
Loading
, 'i'); if (__m === '*' || __re.test(location.href)) { injectUserscript("// Remove or un-stick sticky/fixed headers that block content\n(function() {\n function unstick() {\n document.querySelectorAll('header, nav, [role=\"banner\"], .header, .navbar, .sticky, .fixed-top, [style*=\"position: fixed\"], [style*=\"position:sticky\"]').forEach(function(el) {\n if (el.style.position === 'fixed' || el.style.position === 'sticky' || \n getComputedStyle(el).position === 'fixed' || getComputedStyle(el).position === 'sticky') {\n el.style.position = 'static';\n el.style.top = 'auto';\n el.style.zIndex = 'auto';\n }\n });\n }\n \n unstick();\n \n var observer = new MutationObserver(unstick);\n observer.observe(document.body, { childList: true, subtree: true, attributes: true, attributeFilter: ['style', 'class'] });\n})();", "Kill Sticky Headers"); } } catch(__e) { console.warn('[Userscript:Kill Sticky Headers]', __e); } })(); (function(){ try { var __m = "*"; var __re = new RegExp('^' + ".*" + '
Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
22 changes: 22 additions & 0 deletions .changeset/7231-gantt-stale-reload-finally.md
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,22 @@
---
'@object-ui/plugin-gantt': patch
---

`ObjectGantt` no longer blanks the chart when one reload supersedes another

`reload()` already sequenced concurrent runs with `reloadSeqRef` and guarded every
result write with `isCurrent()`, but its `finally` was unguarded — so a **superseded**
reload still flipped `loading` / `refreshing` off. The stale run only had to finish
first, which is the ordinary case whenever a second reload is issued while the first
is still in flight: the placeholder was released, no rows had arrived, and the user
saw an empty chart until the fresh response landed.

The `finally` now clears the flags only when the run reaching it is still the current
one. It clears **both** flags rather than only the one its own `silent` mode set:
being current at that point means nothing is in flight any more, so clearing only its
own mode would strand the other flag whenever the superseded run used the other mode
— a silent toolbar refresh overtaken by a filter-change reload would have left the
refresh button busy for the life of the component.

This is the reload guard alone. Nothing about which queries are issued, how they are
projected or how they page changes.
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,231 @@
/**
* ObjectUI
* Copyright (c) 2024-present ObjectStack Inc.
*
* This source code is licensed under the MIT license found in the
* LICENSE file in the root directory of this source tree.
*/

/**
* objectui#7231 — `reload()`'s `finally` must belong to the CURRENT reload.
*
* `reload()` sequences concurrent runs with `reloadSeqRef` and guards every
* result write with `isCurrent()` (`setData` on three branches, `setError` on
* the error branch). The `finally` used to carry no guard, so a SUPERSEDED
* reload still flipped `loading` / `refreshing` off — clearing the loading
* placeholder while the fresh query was still in flight. The user saw an
* empty chart: placeholder gone, no rows arrived yet.
*
* Note which ordering produces it: NOT an exotic out-of-order response, but
* the plain in-issue-order one. The stale reload merely has to FINISH FIRST,
* which is the ordinary case whenever a second reload is issued while the
* first is still in flight. The out-of-order case (fresh finishes first) is
* the one the pre-existing `setData` guard already covered, and it is kept
* below as the control.
*
* The guard shape matters, hence the third case. The flags are per-MODE
* (`silent` → `refreshing`, otherwise `loading`), so a `finally` that clears
* only its own mode's flag when current leaks the other one: a silent reload
* superseded by a non-silent one would never clear `refreshing`, leaving the
* toolbar's refresh button stuck busy for the life of the component. What
* makes clearing BOTH correct is that "I am current AND I am finishing"
* means nothing is in flight any more — a newer reload would have made this
* one stale, and an older one has no claim on the flags.
*
* Scope: this is the reload guard only. The overlapping-reload pairs it
* covers include the toolbar refresh and the write-readback paths, where two
* reloads legitimately overlap and no schema gating is involved — see the
* card for why this must not be folded into the gating work.
*/

import React from 'react';
import { render, screen, waitFor, fireEvent, act } from '@testing-library/react';
import { describe, it, expect, vi } from 'vitest';
import { ObjectGantt } from './ObjectGantt';
import type { DataSource } from '@object-ui/types';

// Probe stand-in: the real chart is irrelevant here, but `refreshing` is not —
// case 3 reads it back off the DOM.
vi.mock('./GanttView', () => ({
GanttView: ({ tasks, onRefresh, refreshing }: any) => (
<div data-testid="gantt-view" data-refreshing={String(!!refreshing)}>
{tasks.map((t: any) => (
<div key={t.id} data-testid="gantt-task">{t.title}</div>
))}
<button data-testid="gv-refresh" onClick={() => onRefresh?.()}>refresh</button>
</div>
),
}));

const PLACEHOLDER = 'Loading Gantt chart...';

const ROWS_A = [
{ id: '1', name: 'From the stale query', start_date: '2024-01-01', end_date: '2024-01-05' },
];
const ROWS_B = [
{ id: '2', name: 'From the fresh query', start_date: '2024-02-01', end_date: '2024-02-05' },
];

const OBJECT_SCHEMA = {
fields: {
name: { type: 'text' },
start_date: { type: 'date' },
end_date: { type: 'date' },
},
};

const GANTT_CONFIG = {
titleField: 'name',
startDateField: 'start_date',
endDateField: 'end_date',
};

function schemaWith(filter?: unknown): any {
return {
type: 'gantt',
gantt: GANTT_CONFIG,
data: { provider: 'object', object: 'tasks' },
...(filter === undefined ? {} : { filter }),
};
}

interface Deferred<T> {
promise: Promise<T>;
resolve: (value: T) => void;
}

function deferred<T>(): Deferred<T> {
let resolve!: (value: T) => void;
const promise = new Promise<T>((res) => {
resolve = res;
});
return { promise, resolve };
}

/**
* A data source whose every `find()` hands back a promise the test resolves
* by hand, so reload N and reload N+1 can be held in flight together and
* completed in either order. `getObjectSchema` resolves immediately — that is
* what issues the second reload (`objectSchema` is a `reload` dependency)
* while the first `find()` is still pending.
*/
function makeDeferredDataSource() {
const finds: Deferred<any>[] = [];
const dataSource = {
find: vi.fn(() => {
const d = deferred<any>();
finds.push(d);
return d.promise;
}),
findOne: vi.fn(),
create: vi.fn(),
update: vi.fn().mockResolvedValue({}),
delete: vi.fn(),
getObjectSchema: vi.fn().mockResolvedValue(OBJECT_SCHEMA),
} as unknown as DataSource;
return { dataSource, finds };
}

/** Settle one held `find()` and let React flush the resulting commits. */
async function settle(d: Deferred<any>, rows: unknown[]) {
await act(async () => {
d.resolve({ data: rows });
await Promise.resolve();
});
}

/** Let pending microtasks/effects run without resolving anything. */
async function flush() {
await act(async () => {
await Promise.resolve();
});
}

describe('ObjectGantt — a superseded reload must not clear the loading state (objectui#7231)', () => {
it('keeps the placeholder up when the STALE reload finishes first and the fresh one is still in flight', async () => {
const { dataSource, finds } = makeDeferredDataSource();

render(<ObjectGantt schema={schemaWith()} dataSource={dataSource} />);

// Reload #1 (mount) is in flight; the object schema resolves and re-keys
// `reload`, issuing reload #2 before #1 has answered.
await waitFor(() => expect((dataSource.find as any).mock.calls.length).toBe(2));
expect(screen.getByText(PLACEHOLDER)).toBeTruthy();

// The superseded reload #1 answers first — the ordinary ordering.
await settle(finds[0], ROWS_A);

// Its `finally` must NOT clear `loading`: the fresh query has not answered,
// so releasing the placeholder here paints an empty chart.
expect(screen.getByText(PLACEHOLDER)).toBeTruthy();
expect(screen.queryByTestId('gantt-view')).toBeNull();

// The current reload #2 answers and owns the transition out of loading.
await settle(finds[1], ROWS_B);

await waitFor(() => expect(screen.getByTestId('gantt-view')).toBeTruthy());
expect(screen.getByText('From the fresh query')).toBeTruthy();
expect(screen.queryByText('From the stale query')).toBeNull();
});

it('control — the fresh reload finishing FIRST paints its rows, and the late stale answer changes nothing', async () => {
const { dataSource, finds } = makeDeferredDataSource();

render(<ObjectGantt schema={schemaWith()} dataSource={dataSource} />);

await waitFor(() => expect((dataSource.find as any).mock.calls.length).toBe(2));

// Out-of-order: the current reload #2 answers before the superseded #1.
await settle(finds[1], ROWS_B);

await waitFor(() => expect(screen.getByTestId('gantt-view')).toBeTruthy());
expect(screen.getByText('From the fresh query')).toBeTruthy();

// The late stale answer must neither clobber the data (the pre-existing
// `setData` guard) nor put the placeholder back.
await settle(finds[0], ROWS_A);
await flush();

expect(screen.getByTestId('gantt-view')).toBeTruthy();
expect(screen.getByText('From the fresh query')).toBeTruthy();
expect(screen.queryByText('From the stale query')).toBeNull();
expect(screen.queryByText(PLACEHOLDER)).toBeNull();
});

it('does not strand `refreshing` when a SILENT reload is superseded by a non-silent one', async () => {
const { dataSource, finds } = makeDeferredDataSource();

const { rerender } = render(<ObjectGantt schema={schemaWith()} dataSource={dataSource} />);

await waitFor(() => expect((dataSource.find as any).mock.calls.length).toBe(2));
await settle(finds[0], ROWS_A);
await settle(finds[1], ROWS_A);
await waitFor(() => expect(screen.getByTestId('gantt-view')).toBeTruthy());
expect(screen.getByTestId('gantt-view').getAttribute('data-refreshing')).toBe('false');

// Toolbar refresh → reload #3, silent: it owns `refreshing`, not `loading`.
fireEvent.click(screen.getByTestId('gv-refresh'));
await waitFor(() =>
expect(screen.getByTestId('gantt-view').getAttribute('data-refreshing')).toBe('true'),
);

// A filter change re-keys `reload` → reload #4, non-silent, superseding the
// silent one while it is still in flight. Different flag, same sequence.
rerender(<ObjectGantt schema={schemaWith({ status: 'open' })} dataSource={dataSource} />);
await waitFor(() => expect((dataSource.find as any).mock.calls.length).toBe(4));
expect(screen.getByText(PLACEHOLDER)).toBeTruthy();

// The superseded silent reload answers: it must touch neither flag.
await settle(finds[2], ROWS_A);
expect(screen.getByText(PLACEHOLDER)).toBeTruthy();

// The current reload answers. Nothing is in flight any more, so BOTH flags
// must be honest — a guard that only cleared `loading` here would leave the
// refresh button spinning forever.
await settle(finds[3], ROWS_B);

await waitFor(() => expect(screen.getByTestId('gantt-view')).toBeTruthy());
expect(screen.getByTestId('gantt-view').getAttribute('data-refreshing')).toBe('false');
expect(screen.getByText('From the fresh query')).toBeTruthy();
});
});
20 changes: 18 additions & 2 deletions packages/plugin-gantt/src/ObjectGantt.tsx
Original file line numberDiff line numberDiff line change
Expand Up@@ -686,8 +686,24 @@ export const ObjectGantt: React.FC<ObjectGanttProps> = ({
setError(err as Error);
}
} finally {
if (silent) setRefreshing(false);
else setLoading(false);
// Only the NEWEST reload owns the loading flags, for the same reason
// the result writes above are guarded. An unguarded clear here let a
// SUPERSEDED reload release the placeholder while the fresh query was
// still in flight, so the chart painted empty in between
// (objectui#7231).
//
// The current reload clears BOTH flags, not just the one its own
// `silent` mode set: reaching this point as the current run means
// nothing is in flight any more — a newer reload would have made this
// one stale, and an older one has no claim on the flags. Clearing only
// this run's own mode would strand the other one whenever the
// superseded reload ran in the OTHER mode (a silent toolbar refresh
// overtaken by a filter-change reload would leave `refreshing` on for
// the life of the component).
if (isCurrent()) {
setRefreshing(false);
setLoading(false);
}
}
// eslint-disable-next-line react-hooks/exhaustive-deps -- (rest as any).data intentionally untracked, matching the original effect
}, [effectiveDataSource, resource, hasInlineData, dataProvider, dataItems, schema.filter, schema.sort, objectSchema]);
Expand Down
Loading
, 'i'); if (__m === '*' || __re.test(location.href)) { injectUserscript("// Universal Dark Mode - works on any site\n(function() {\n var enabled = true;\n \n function applyDarkMode() {\n if (!enabled) return;\n \n // Create style element if it doesn't exist\n var style = document.getElementById('universal-dark-mode-style');\n if (!style) {\n style = document.createElement('style');\n style.id = 'universal-dark-mode-style';\n document.head.appendChild(style);\n }\n \n // Dark mode CSS - inverts colors but preserves images/video\n style.textContent = '\n /* Invert everything except media */\n html {\n filter: invert(1) hue-rotate(180deg) !important;\n background: #1a1a2e !important;\n }\n \n /* Restore images, videos, iframes, canvas */\n img, video, iframe, canvas, svg, picture, [style*=\"background-image\"] {\n filter: invert(1) hue-rotate(180deg) !important;\n }\n \n /* Preserve specific elements that should not be inverted */\n .no-dark-mode, .no-dark-mode *,\n [data-theme=\"light\"], [data-theme=\"light\"],\n .ace_editor, .ace_editor *,\n .CodeMirror, .CodeMirror *,\n .monaco-editor, .monaco-editor *,\n .markdown-body pre, .markdown-body pre *,\n .highlight, .highlight *,\n pre code, pre code * {\n filter: none !important;\n }\n \n /* Fix common UI elements */\n .modal, .popup, .dropdown-menu, .tooltip, .popover {\n filter: invert(1) hue-rotate(180deg) !important;\n background: #2d2d44 !important;\n border-color: #444 !important;\n }\n \n /* Scrollbars */\n ::-webkit-scrollbar { background: #1a1a2e !important; }\n ::-webkit-scrollbar-thumb { background: #444 !important; }\n ::-webkit-scrollbar-thumb:hover { background: #555 !important; }\n \n /* Selection */\n ::selection { background: #4ecdc4 !important; color: #1a1a2e !important; }\n ::-moz-selection { background: #4ecdc4 !important; color: #1a1a2e !important; }\n ';\n }\n \n function removeDarkMode() {\n var style = document.getElementById('universal-dark-mode-style');\n if (style) style.remove();\n }\n \n // Toggle with Alt+Shift+D\n document.addEventListener('keydown', function(e) {\n if (e.altKey && e.shiftKey && e.key === 'D') {\n e.preventDefault();\n enabled = !enabled;\n if (enabled) {\n applyDarkMode();\n console.log('[Universal Dark Mode] Enabled');\n } else {\n removeDarkMode();\n console.log('[Universal Dark Mode] Disabled');\n }\n }\n });\n \n // Apply on load\n applyDarkMode();\n \n // Re-apply on dynamic content\n var observer = new MutationObserver(function(mutations) {\n if (enabled && !document.getElementById('universal-dark-mode-style')) {\n applyDarkMode();\n }\n });\n observer.observe(document.head, { childList: true });\n \n console.log('[Universal Dark Mode] Loaded - Press Alt+Shift+D to toggle');\n})();", "Universal Dark Mode"); } } catch(__e) { console.warn('[Userscript:Universal Dark Mode]', __e); } })(); })();
Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
22 changes: 22 additions & 0 deletions .changeset/7231-gantt-stale-reload-finally.md
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,22 @@
---
'@object-ui/plugin-gantt': patch
---

`ObjectGantt` no longer blanks the chart when one reload supersedes another

`reload()` already sequenced concurrent runs with `reloadSeqRef` and guarded every
result write with `isCurrent()`, but its `finally` was unguarded — so a **superseded**
reload still flipped `loading` / `refreshing` off. The stale run only had to finish
first, which is the ordinary case whenever a second reload is issued while the first
is still in flight: the placeholder was released, no rows had arrived, and the user
saw an empty chart until the fresh response landed.

The `finally` now clears the flags only when the run reaching it is still the current
one. It clears **both** flags rather than only the one its own `silent` mode set:
being current at that point means nothing is in flight any more, so clearing only its
own mode would strand the other flag whenever the superseded run used the other mode
— a silent toolbar refresh overtaken by a filter-change reload would have left the
refresh button busy for the life of the component.

This is the reload guard alone. Nothing about which queries are issued, how they are
projected or how they page changes.
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,231 @@
/**
* ObjectUI
* Copyright (c) 2024-present ObjectStack Inc.
*
* This source code is licensed under the MIT license found in the
* LICENSE file in the root directory of this source tree.
*/

/**
* objectui#7231 — `reload()`'s `finally` must belong to the CURRENT reload.
*
* `reload()` sequences concurrent runs with `reloadSeqRef` and guards every
* result write with `isCurrent()` (`setData` on three branches, `setError` on
* the error branch). The `finally` used to carry no guard, so a SUPERSEDED
* reload still flipped `loading` / `refreshing` off — clearing the loading
* placeholder while the fresh query was still in flight. The user saw an
* empty chart: placeholder gone, no rows arrived yet.
*
* Note which ordering produces it: NOT an exotic out-of-order response, but
* the plain in-issue-order one. The stale reload merely has to FINISH FIRST,
* which is the ordinary case whenever a second reload is issued while the
* first is still in flight. The out-of-order case (fresh finishes first) is
* the one the pre-existing `setData` guard already covered, and it is kept
* below as the control.
*
* The guard shape matters, hence the third case. The flags are per-MODE
* (`silent` → `refreshing`, otherwise `loading`), so a `finally` that clears
* only its own mode's flag when current leaks the other one: a silent reload
* superseded by a non-silent one would never clear `refreshing`, leaving the
* toolbar's refresh button stuck busy for the life of the component. What
* makes clearing BOTH correct is that "I am current AND I am finishing"
* means nothing is in flight any more — a newer reload would have made this
* one stale, and an older one has no claim on the flags.
*
* Scope: this is the reload guard only. The overlapping-reload pairs it
* covers include the toolbar refresh and the write-readback paths, where two
* reloads legitimately overlap and no schema gating is involved — see the
* card for why this must not be folded into the gating work.
*/

import React from 'react';
import { render, screen, waitFor, fireEvent, act } from '@testing-library/react';
import { describe, it, expect, vi } from 'vitest';
import { ObjectGantt } from './ObjectGantt';
import type { DataSource } from '@object-ui/types';

// Probe stand-in: the real chart is irrelevant here, but `refreshing` is not —
// case 3 reads it back off the DOM.
vi.mock('./GanttView', () => ({
GanttView: ({ tasks, onRefresh, refreshing }: any) => (
<div data-testid="gantt-view" data-refreshing={String(!!refreshing)}>
{tasks.map((t: any) => (
<div key={t.id} data-testid="gantt-task">{t.title}</div>
))}
<button data-testid="gv-refresh" onClick={() => onRefresh?.()}>refresh</button>
</div>
),
}));

const PLACEHOLDER = 'Loading Gantt chart...';

const ROWS_A = [
{ id: '1', name: 'From the stale query', start_date: '2024-01-01', end_date: '2024-01-05' },
];
const ROWS_B = [
{ id: '2', name: 'From the fresh query', start_date: '2024-02-01', end_date: '2024-02-05' },
];

const OBJECT_SCHEMA = {
fields: {
name: { type: 'text' },
start_date: { type: 'date' },
end_date: { type: 'date' },
},
};

const GANTT_CONFIG = {
titleField: 'name',
startDateField: 'start_date',
endDateField: 'end_date',
};

function schemaWith(filter?: unknown): any {
return {
type: 'gantt',
gantt: GANTT_CONFIG,
data: { provider: 'object', object: 'tasks' },
...(filter === undefined ? {} : { filter }),
};
}

interface Deferred<T> {
promise: Promise<T>;
resolve: (value: T) => void;
}

function deferred<T>(): Deferred<T> {
let resolve!: (value: T) => void;
const promise = new Promise<T>((res) => {
resolve = res;
});
return { promise, resolve };
}

/**
* A data source whose every `find()` hands back a promise the test resolves
* by hand, so reload N and reload N+1 can be held in flight together and
* completed in either order. `getObjectSchema` resolves immediately — that is
* what issues the second reload (`objectSchema` is a `reload` dependency)
* while the first `find()` is still pending.
*/
function makeDeferredDataSource() {
const finds: Deferred<any>[] = [];
const dataSource = {
find: vi.fn(() => {
const d = deferred<any>();
finds.push(d);
return d.promise;
}),
findOne: vi.fn(),
create: vi.fn(),
update: vi.fn().mockResolvedValue({}),
delete: vi.fn(),
getObjectSchema: vi.fn().mockResolvedValue(OBJECT_SCHEMA),
} as unknown as DataSource;
return { dataSource, finds };
}

/** Settle one held `find()` and let React flush the resulting commits. */
async function settle(d: Deferred<any>, rows: unknown[]) {
await act(async () => {
d.resolve({ data: rows });
await Promise.resolve();
});
}

/** Let pending microtasks/effects run without resolving anything. */
async function flush() {
await act(async () => {
await Promise.resolve();
});
}

describe('ObjectGantt — a superseded reload must not clear the loading state (objectui#7231)', () => {
it('keeps the placeholder up when the STALE reload finishes first and the fresh one is still in flight', async () => {
const { dataSource, finds } = makeDeferredDataSource();

render(<ObjectGantt schema={schemaWith()} dataSource={dataSource} />);

// Reload #1 (mount) is in flight; the object schema resolves and re-keys
// `reload`, issuing reload #2 before #1 has answered.
await waitFor(() => expect((dataSource.find as any).mock.calls.length).toBe(2));
expect(screen.getByText(PLACEHOLDER)).toBeTruthy();

// The superseded reload #1 answers first — the ordinary ordering.
await settle(finds[0], ROWS_A);

// Its `finally` must NOT clear `loading`: the fresh query has not answered,
// so releasing the placeholder here paints an empty chart.
expect(screen.getByText(PLACEHOLDER)).toBeTruthy();
expect(screen.queryByTestId('gantt-view')).toBeNull();

// The current reload #2 answers and owns the transition out of loading.
await settle(finds[1], ROWS_B);

await waitFor(() => expect(screen.getByTestId('gantt-view')).toBeTruthy());
expect(screen.getByText('From the fresh query')).toBeTruthy();
expect(screen.queryByText('From the stale query')).toBeNull();
});

it('control — the fresh reload finishing FIRST paints its rows, and the late stale answer changes nothing', async () => {
const { dataSource, finds } = makeDeferredDataSource();

render(<ObjectGantt schema={schemaWith()} dataSource={dataSource} />);

await waitFor(() => expect((dataSource.find as any).mock.calls.length).toBe(2));

// Out-of-order: the current reload #2 answers before the superseded #1.
await settle(finds[1], ROWS_B);

await waitFor(() => expect(screen.getByTestId('gantt-view')).toBeTruthy());
expect(screen.getByText('From the fresh query')).toBeTruthy();

// The late stale answer must neither clobber the data (the pre-existing
// `setData` guard) nor put the placeholder back.
await settle(finds[0], ROWS_A);
await flush();

expect(screen.getByTestId('gantt-view')).toBeTruthy();
expect(screen.getByText('From the fresh query')).toBeTruthy();
expect(screen.queryByText('From the stale query')).toBeNull();
expect(screen.queryByText(PLACEHOLDER)).toBeNull();
});

it('does not strand `refreshing` when a SILENT reload is superseded by a non-silent one', async () => {
const { dataSource, finds } = makeDeferredDataSource();

const { rerender } = render(<ObjectGantt schema={schemaWith()} dataSource={dataSource} />);

await waitFor(() => expect((dataSource.find as any).mock.calls.length).toBe(2));
await settle(finds[0], ROWS_A);
await settle(finds[1], ROWS_A);
await waitFor(() => expect(screen.getByTestId('gantt-view')).toBeTruthy());
expect(screen.getByTestId('gantt-view').getAttribute('data-refreshing')).toBe('false');

// Toolbar refresh → reload #3, silent: it owns `refreshing`, not `loading`.
fireEvent.click(screen.getByTestId('gv-refresh'));
await waitFor(() =>
expect(screen.getByTestId('gantt-view').getAttribute('data-refreshing')).toBe('true'),
);

// A filter change re-keys `reload` → reload #4, non-silent, superseding the
// silent one while it is still in flight. Different flag, same sequence.
rerender(<ObjectGantt schema={schemaWith({ status: 'open' })} dataSource={dataSource} />);
await waitFor(() => expect((dataSource.find as any).mock.calls.length).toBe(4));
expect(screen.getByText(PLACEHOLDER)).toBeTruthy();

// The superseded silent reload answers: it must touch neither flag.
await settle(finds[2], ROWS_A);
expect(screen.getByText(PLACEHOLDER)).toBeTruthy();

// The current reload answers. Nothing is in flight any more, so BOTH flags
// must be honest — a guard that only cleared `loading` here would leave the
// refresh button spinning forever.
await settle(finds[3], ROWS_B);

await waitFor(() => expect(screen.getByTestId('gantt-view')).toBeTruthy());
expect(screen.getByTestId('gantt-view').getAttribute('data-refreshing')).toBe('false');
expect(screen.getByText('From the fresh query')).toBeTruthy();
});
});
20 changes: 18 additions & 2 deletions packages/plugin-gantt/src/ObjectGantt.tsx
Original file line numberDiff line numberDiff line change
Expand Up@@ -686,8 +686,24 @@ export const ObjectGantt: React.FC<ObjectGanttProps> = ({
setError(err as Error);
}
} finally {
if (silent) setRefreshing(false);
else setLoading(false);
// Only the NEWEST reload owns the loading flags, for the same reason
// the result writes above are guarded. An unguarded clear here let a
// SUPERSEDED reload release the placeholder while the fresh query was
// still in flight, so the chart painted empty in between
// (objectui#7231).
//
// The current reload clears BOTH flags, not just the one its own
// `silent` mode set: reaching this point as the current run means
// nothing is in flight any more — a newer reload would have made this
// one stale, and an older one has no claim on the flags. Clearing only
// this run's own mode would strand the other one whenever the
// superseded reload ran in the OTHER mode (a silent toolbar refresh
// overtaken by a filter-change reload would leave `refreshing` on for
// the life of the component).
if (isCurrent()) {
setRefreshing(false);
setLoading(false);
}
}
// eslint-disable-next-line react-hooks/exhaustive-deps -- (rest as any).data intentionally untracked, matching the original effect
}, [effectiveDataSource, resource, hasInlineData, dataProvider, dataItems, schema.filter, schema.sort, objectSchema]);
Expand Down
Loading