Skip to content
Closed
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
20 changes: 10 additions & 10 deletions .github/file-filters.yml
Original file line number Diff line number Diff line change
@@ -1,35 +1,35 @@
# This is used by the action https://github.com/dorny/paths-filter

sentry_frontend_workflow_file: &sentry_frontend_workflow_file
sentry_specific_workflow: &sentry_specific_workflow
- added|modified: '.github/workflows/frontend.yml'
sentry_specific_test_files: &sentry_specific_test_files
- added|modified: 'tests/js/**/*'
- added|deleted|modified: 'fixtures/search-syntax/**/*'

# Provides list of changed files to test (jest)
# getsentry/sentry does not use the list directly, instead we shard tests inside jest.config.js
# getsentry/sentry does not use this directly, instead we shard tests inside jest.config.js
testable_modified: &testable_modified
- '!**/*.generated.ts'
- added|modified: 'package.json'
- added|modified: 'static/**/*.{ts,tsx,js,jsx,mjs}'
- added|modified: 'tests/js/**/*'
- added|deleted|modified: 'fixtures/search-syntax/**/*'
- added|modified: 'static/**/*.[tj]{s,sx}'
- *sentry_specific_test_files

# Trigger for when we must run full tests (jest)
testable_rules_changed: &testable_rules_changed
- *sentry_frontend_workflow_file
- *sentry_specific_workflow
- added|modified: '.github/file-filters.yml'
- added|modified: 'jest.config.ts'

# Trigger whether to run tsc or not
# There's no "rules_changed" for this, because we run it for all files always
# getsentry/sentry does not use this directly, instead frontend_all is a superset to trigger tsc
typecheckable_rules_changed: &typecheckable_rules_changed
- *sentry_frontend_workflow_file
- *sentry_specific_workflow
- added|modified: '.github/file-filters.yml'
- added|deleted|modified: '**/tsconfig*.json'
- added|deleted|modified: 'static/**/*.{ts,tsx,js,jsx,mjs}'
- added|deleted|modified: 'static/**/*.[tj]{s,sx}'

# Trigger to apply the 'Scope: Frontend' label to PRs
frontend_all: &frontend_all
- *sentry_frontend_workflow_file
- added|modified: '**/*.{ts,tsx,js,jsx,mjs}'
- added|modified: 'static/**/*.{less,json,yml,md,mdx}'
- added|modified: '{.volta,vercel,tsconfig,biome,package}.json'
Expand Down
2 changes: 1 addition & 1 deletion .github/workflows/frontend.yml
Original file line number Diff line number Diff line change
Expand Up @@ -25,8 +25,8 @@ jobs:
# Map a step output to a job output
outputs:
testable_modified: ${{ steps.changes.outputs.testable_modified }}
testable_modified_files: ${{ steps.changes.outputs.testable_modified_files }}
testable_rules_changed: ${{ steps.changes.outputs.testable_rules_changed }}
typecheckable_rules_changed: ${{ steps.changes.outputs.typecheckable_rules_changed }}
frontend_all: ${{ steps.changes.outputs.frontend_all }}
steps:
- uses: actions/checkout@692973e3d937129bcbf40652eb9f2f61becf3332 # v4.1.7
Expand Down
6 changes: 0 additions & 6 deletions .pre-commit-config.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -135,12 +135,6 @@ repos:
stages: [pre-push]
entry: bash -c 'if [ -n "${SENTRY_KNIP_PRE_PUSH:-}" ]; then exec ./node_modules/.bin/knip; fi' --

- id: gen-ts-api-urls
name: gen-ts-api-urls
language: system
files: /urls\.py$
entry: python3 -m tools.api_urls_to_typescript

- repo: https://github.com/pre-commit/pygrep-hooks
rev: v1.10.0
hooks:
Expand Down
3 changes: 3 additions & 0 deletions static/app/api/apiDefinition.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,3 @@
type KnownApiUrls = ['/projects/$orgSlug/$projectSlug/releases/$releaseVersion/'];

export type MaybeApiPath = KnownApiUrls[number] | (string & {});
Original file line number Diff line number Diff line change
Expand Up @@ -4,13 +4,14 @@ import {expectTypeOf} from 'expect-type';
import {renderHook, waitFor} from 'sentry-test/reactTestingLibrary';

import type {ApiResult} from 'sentry/api';
import {apiOptions, selectWithHeaders} from 'sentry/utils/api/apiOptions';
import {
DEFAULT_QUERY_CLIENT_CONFIG,
QueryClient,
QueryClientProvider,
} from 'sentry/utils/queryClient';

import {apiOptions, selectWithHeaders} from './apiOptions';

type Promisable<T> = T | Promise<T>;
type QueryFunctionResult<T> = Promisable<ApiResult<T>>;

Expand All @@ -21,41 +22,44 @@ const wrapper = ({children}: {children?: React.ReactNode}) => (
);

describe('apiOptions', () => {
it('should encode path parameters correctly', () => {
afterEach(() => {
MockApiClient.clearMockResponses();
});
test('should encode path parameters correctly', () => {
const options = apiOptions.as<unknown>()(
'/organizations/$organizationIdOrSlug/releases/$version/',
'/projects/$orgSlug/$projectSlug/releases/$releaseVersion/',
{
staleTime: 0,
path: {
organizationIdOrSlug: 'my-org',
version: 'v 1.0.0',
orgSlug: 'my-org',
projectSlug: 'my-project',
releaseVersion: 'v 1.0.0',
},
}
);

expect(options.queryKey[0]).toBe('/organizations/my-org/releases/v%201.0.0/');
expect(options.queryKey[0]).toBe('/projects/my-org/my-project/releases/v%201.0.0/');
});

it('should not include empty options in queryKey', () => {
const options = apiOptions.as<unknown>()('/api-tokens/$tokenId/', {
test('should not include empty options in queryKey', () => {
const options = apiOptions.as<unknown>()('/projects/$id/', {
staleTime: 0,
path: {tokenId: '123'},
path: {id: '123'},
});

expect(options.queryKey).toEqual(['/api-tokens/123/']);
expect(options.queryKey).toEqual(['/projects/123/']);
});

it('should stringify number path params', () => {
const options = apiOptions.as<unknown>()('/api-tokens/$tokenId/', {
test('should stringify number path params', () => {
const options = apiOptions.as<unknown>()('/items/$id/', {
staleTime: 0,
path: {tokenId: 123},
path: {id: 123},
});

expect(options.queryKey[0]).toBe('/api-tokens/123/');
expect(options.queryKey[0]).toBe('/items/123/');
});

it('should not do accidental replacements', () => {
// @ts-expect-error Using a sample path, not a real one
test('should not do accidental replacements', () => {
const options = apiOptions.as<unknown>()('/projects/$id1/$id', {
staleTime: 0,
path: {id: '123', id1: '456'},
Expand All @@ -64,21 +68,21 @@ describe('apiOptions', () => {
expect(options.queryKey).toEqual(['/projects/456/123']);
});

it('should allow skipToken as path', () => {
function getOptions(tokenId: string | null) {
return apiOptions.as<unknown>()('/api-tokens/$tokenId/', {
test('should allow skipToken as path', () => {
function getOptions(id: string | null) {
return apiOptions.as<unknown>()('/projects/$id/', {
staleTime: 0,
path: tokenId ? {tokenId} : skipToken,
path: id ? {id} : skipToken,
});
}

expect(getOptions('123').queryFn).toEqual(expect.any(Function));
expect(getOptions('123').queryKey).toEqual(['/api-tokens/123/']);
expect(getOptions('123').queryKey).toEqual(['/projects/123/']);
expect(getOptions(null).queryFn).toEqual(skipToken);
expect(getOptions(null).queryKey).toEqual(['/api-tokens/$tokenId/']);
expect(getOptions(null).queryKey).toEqual(['/projects/$id/']);
});

it('should extract content data per default', async () => {
test('should extract content data per default', async () => {
const options = apiOptions.as<string[]>()('/projects/', {
staleTime: 0,
});
Expand All @@ -95,7 +99,7 @@ describe('apiOptions', () => {
expect(result.current.data).toEqual(['Project 1', 'Project 2']);
});

it('should extract headers', async () => {
test('should extract headers', async () => {
const options = apiOptions.as<string[]>()('/projects/', {
staleTime: 0,
});
Expand Down Expand Up @@ -130,37 +134,47 @@ describe('apiOptions', () => {
});

describe('types', () => {
it('should always require staleTime', () => {
test('should always require staleTime', () => {
// @ts-expect-error staleTime is required
apiOptions.as<unknown>()('/projects/$orgSlug/', {path: {orgSlug: 'my-org'}});
// @ts-expect-error staleTime is required
apiOptions.as<unknown>()('/projects/', {});
});

it('should not allow invalid/excess path parameters', () => {
const options = apiOptions.as<never>()('/api-tokens/$tokenId/', {
test('should not allow invalid path parameters', () => {
const options = apiOptions.as<never>()('/projects/$orgSlug/', {
staleTime: 0,
// @ts-expect-error Invalid path parameter
path: {orgSlug: 'my-org', invalidParam: 'invalid'},
});

expectTypeOf(options.queryFn).returns.toEqualTypeOf<QueryFunctionResult<never>>();
});

test('should not allow excess path parameters', () => {
const options = apiOptions.as<never>()('/projects/$orgSlug/', {
staleTime: 0,
// @ts-expect-error Missing required path parameter
path: {tokenId: 'my-org', invalidParam: 'invalid'},
// @ts-expect-error Excess path parameter
path: {orgSlug: 'my-org', extraParam: 'extra'},
});

expectTypeOf(options.queryFn).returns.toEqualTypeOf<QueryFunctionResult<never>>();
});

it('should require path params for paths with parameters', () => {
test('should require path params for paths with parameters', () => {
expect(() => {
const options = apiOptions.as<never>()('/api-tokens/$tokenId/', {
const options = apiOptions.as<never>()('/projects/$orgSlug/', {
staleTime: 0,
// @ts-expect-error Missing required path parameter
path: {},
});

expectTypeOf(options.queryFn).returns.toEqualTypeOf<QueryFunctionResult<never>>();
}).toThrow('Missing path param: tokenId');
}).toThrow('Missing path param: orgSlug');
});

it('should not allow empty path parameters for paths without parameters', () => {
const options = apiOptions.as<never>()('/api-tokens/', {
test('should not allow empty path parameters for paths without parameters', () => {
const options = apiOptions.as<never>()('/projects/', {
staleTime: 0,
// @ts-expect-error Empty path parameters not allowed
path: {},
Expand All @@ -169,32 +183,31 @@ describe('apiOptions', () => {
expectTypeOf(options.queryFn).returns.toEqualTypeOf<QueryFunctionResult<never>>();
});

it('should not need path params for paths without parameters', () => {
const options = apiOptions.as<never>()('/api-tokens/', {
test('should not need path params for paths without parameters', () => {
const options = apiOptions.as<never>()('/projects/', {
staleTime: 0,
});

expectTypeOf(options.queryFn).returns.toEqualTypeOf<QueryFunctionResult<never>>();
});

it('should allow string or number path parameters', () => {
const options = apiOptions.as<never>()('/api-tokens/$tokenId/', {
test('should allow string or number path parameters', () => {
const options = apiOptions.as<never>()('/items/$id/', {
staleTime: 0,
path: {tokenId: 123},
path: {id: 123},
});

expectTypeOf(options.queryFn).returns.toEqualTypeOf<QueryFunctionResult<never>>();

const options2 = apiOptions.as<never>()('/api-tokens/$tokenId/', {
const options2 = apiOptions.as<never>()('/items/$id/', {
staleTime: 0,
path: {tokenId: 'abc'},
path: {id: 'abc'},
});

expectTypeOf(options2.queryFn).returns.toEqualTypeOf<QueryFunctionResult<never>>();
});

it('should default to never for unknown API paths', () => {
// @ts-expect-error Unknown API path
test('should default to never for unknown API paths', () => {
const options = apiOptions.as<never>()('/unknown/$param/', {
staleTime: 0,
path: {param: 'value'},
Expand All @@ -203,17 +216,33 @@ describe('apiOptions', () => {
expectTypeOf(options.queryFn).returns.toEqualTypeOf<QueryFunctionResult<never>>();
});

it('should allow providing manual data type', () => {
const options = apiOptions.as<number>()('/api-tokens/$tokenId/', {
test('should allow providing manual data type', () => {
const options = apiOptions.as<number>()('/foo/$bar', {
staleTime: 0,
path: {tokenId: 'abc'},
path: {bar: 'baz'},
});

expectTypeOf(options.queryFn).returns.toEqualTypeOf<QueryFunctionResult<number>>();
});

it('should disallow unknown path if there are no path params', () => {
const options = apiOptions.as<number>()('/api-tokens/', {
test('manual data type should override even for known api urls', () => {
const options = apiOptions.as<number>()(
'/projects/$orgSlug/$projectSlug/releases/$releaseVersion/',
{
staleTime: 0,
path: {
orgSlug: 'my-org',
projectSlug: 'my-project',
releaseVersion: 'v1.0.0',
},
}
);

expectTypeOf(options.queryFn).returns.toEqualTypeOf<QueryFunctionResult<number>>();
});

test('should disallow path if there are no path params', () => {
const options = apiOptions.as<number>()('/foo', {
staleTime: 0,
// @ts-expect-error Path is not allowed when there are no path params
path: {bar: 'baz'},
Expand All @@ -222,10 +251,10 @@ describe('apiOptions', () => {
expectTypeOf(options.queryFn).returns.toEqualTypeOf<QueryFunctionResult<number>>();
});

it('should have a default select that extracts content', () => {
const options = apiOptions.as<number>()('/api-tokens/$tokenId/', {
test('should have a default select that extracts content', () => {
const options = apiOptions.as<number>()('/items/$id/', {
staleTime: 0,
path: {tokenId: 123},
path: {id: 123},
});

expectTypeOf(options.select).returns.toEqualTypeOf<number>();
Expand Down
Original file line number Diff line number Diff line change
@@ -1,15 +1,10 @@
import {queryOptions, skipToken} from '@tanstack/react-query';
import type {SkipToken} from '@tanstack/react-query';
import {queryOptions, skipToken, type SkipToken} from '@tanstack/react-query';

import type {ApiResult} from 'sentry/api';
import getApiUrl from 'sentry/utils/api/getApiUrl';
import type {ExtractPathParams, OptionalPathParams} from 'sentry/utils/api/getApiUrl';
import type {KnownGetsentryApiUrls} from 'sentry/utils/api/knownGetsentryApiUrls';
import type {KnownSentryApiUrls} from 'sentry/utils/api/knownSentryApiUrls.generated';
import type {QueryKeyEndpointOptions} from 'sentry/utils/queryClient';
import {fetchDataQuery} from 'sentry/utils/queryClient';
import {fetchDataQuery, type QueryKeyEndpointOptions} from 'sentry/utils/queryClient';

type KnownApiUrls = KnownGetsentryApiUrls | KnownSentryApiUrls;
import type {MaybeApiPath} from './apiDefinition';
import {getApiUrl, type ExtractPathParams, type OptionalPathParams} from './getApiUrl';

type Options = QueryKeyEndpointOptions & {staleTime: number};

Expand All @@ -34,7 +29,7 @@ export const selectWithHeaders =

function _apiOptions<
TManualData = never,
TApiPath extends KnownApiUrls = KnownApiUrls,
TApiPath extends MaybeApiPath = MaybeApiPath,
// todo: infer the actual data type from the ApiMapping
TActualData = TManualData,
>(
Expand Down Expand Up @@ -73,7 +68,7 @@ function _apiOptions<
export const apiOptions = {
as:
<TManualData>() =>
<TApiPath extends KnownApiUrls = KnownApiUrls>(
<TApiPath extends MaybeApiPath = MaybeApiPath>(
path: TApiPath,
options: Options & PathParamOptions<TApiPath>
) =>
Expand Down
Loading
Loading