Skip to content

ref(tsc): move event-timeseries endpoint to apiOptions#113883

Merged
TkDodo merged 4 commits intomasterfrom
tkdodo/ref/events-timeseries-to-apiOptions
Apr 27, 2026
Merged

ref(tsc): move event-timeseries endpoint to apiOptions#113883
TkDodo merged 4 commits intomasterfrom
tkdodo/ref/events-timeseries-to-apiOptions

Conversation

@TkDodo
Copy link
Copy Markdown
Collaborator

@TkDodo TkDodo commented Apr 24, 2026

No description provided.

@github-actions github-actions Bot added the Scope: Frontend Automatically applied to PRs that change frontend components label Apr 24, 2026
@TkDodo TkDodo marked this pull request as ready for review April 24, 2026 07:41
@TkDodo TkDodo requested review from a team as code owners April 24, 2026 07:41
@TkDodo TkDodo requested a review from a team April 24, 2026 07:41
Comment on lines +181 to +185
if (!q?.data) {
return;
}

const responseData = q.data[0];
const responseData = q.data;
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is this related to type inference, or why do we no longer check for the 0th index truthyness?

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

the result of q.data used to come directly from fetchDataQuery, which returned:

export type ApiResult<Data = any> = [
data: Data,
statusText: string | undefined,
resp: ResponseMeta | undefined,
];

[0] was the actual data. Now that we use apiOptions, we internally select .json out of it if no select is specified:

select: selectJson,

const selectJson = <TData>(data: ApiResponse<TData>) => data.json;

so q.data is our data, same as q.data[0] was before.

Copy link
Copy Markdown
Member

@priscilawebdev priscilawebdev left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

👏

Comment on lines +183 to +184
enabled: enabled && (hasCustomPageFilters ? true : arePageFiltersReady),
...options.queryOptions,
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We have got 2x enabled props here (top-level enabled and queryOptions.enabled), and the spread order means queryOptions.enabled overwrites the arePageFiltersReady gate. Not introduced by this PR, but worth a follow-up: probably best to drop one of them

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

yes I was confused about this as well 🫠

@TkDodo TkDodo merged commit 281ebea into master Apr 27, 2026
65 checks passed
@TkDodo TkDodo deleted the tkdodo/ref/events-timeseries-to-apiOptions branch April 27, 2026 12:14
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Scope: Frontend Automatically applied to PRs that change frontend components

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants