Skip to content

Typechecking fixes #77

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
Bentroen merged 14 commits into from
Jan 16, 2026
Merged

Typechecking fixes #77

Show file tree
Hide file tree
Changes from 4 commits
Commits
Show all changes
14 commits
Select commit Hold shift + click to select a range
3fa1910
update dev script in package.json to include webpack
tomast1337 Jan 11, 2026
c332eb5
refactor: update TypeScript configurations and improve type handling …
tomast1337 Jan 11, 2026
71f73f1
Merge branch 'develop' of github.com:OpenNBS/NoteBlockWorld into develop
tomast1337 Jan 11, 2026
57c969c
chore(deps): update Next.js and related packages to version 16.0.10
tomast1337 Jan 11, 2026
ed39809
chore: add `@nbw/thumbnail/node` type defs on backend
Bentroen Jan 15, 2026
d34865f
chore(types): change backend's `moduleResolution` to `bundler`
Bentroen Jan 15, 2026
58a90d8
chore(types): remove type definitions for `@nbw/thumbnail/node`
Bentroen Jan 15, 2026
7429301
refactor(types): remove redundant ternary operator in song query `sor…
Bentroen Jan 15, 2026
c5a22d5
refactor(types): remove catch-all types for `jwtService.signAsync` call
Bentroen Jan 15, 2026
8e43122
fix(config): parse duration env configs on startup and coerce to `ms.…
Bentroen Jan 15, 2026
bf1acef
fix(config) use `config.getOrThrow` to simplify logic in `AuthModule`
Bentroen Jan 15, 2026
98c472b
fix(config): validator's `defaultMessage` not being shown on env vali…
Bentroen Jan 15, 2026
ba1d3b9
chore(types): add TODO for improving typing on GitHub login strategy
Bentroen Jan 15, 2026
b25030b
test(song): remove duplicate authorization tests
Bentroen Jan 16, 2026
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
2 changes: 1 addition & 1 deletion apps/backend/src/auth/auth.service.spec.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -192,7 +192,7 @@ describe('AuthService', () => {
const refreshToken = 'refresh-token';

spyOn(jwtService, 'signAsync').mockImplementation(
(payload, options: any) => {
(payload: any, options: any) => {
if (options.secret === 'test-jwt-secret') {
return Promise.resolve(accessToken);
} else if (options.secret === 'test-jwt-refresh-secret') {
Expand Down
22 changes: 14 additions & 8 deletions apps/backend/src/auth/auth.service.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -171,14 +171,20 @@ export class AuthService {

public async createJwtPayload(payload: TokenPayload): Promise<Tokens> {
const [accessToken, refreshToken] = await Promise.all([
this.jwtService.signAsync(payload, {
secret: this.JWT_SECRET,
expiresIn: this.JWT_EXPIRES_IN,
}),
this.jwtService.signAsync(payload, {
secret: this.JWT_REFRESH_SECRET,
expiresIn: this.JWT_REFRESH_EXPIRES_IN,
}),
this.jwtService.signAsync(
payload as object,
{
secret: this.JWT_SECRET,
expiresIn: this.JWT_EXPIRES_IN,
} as any,
),
this.jwtService.signAsync(
payload as object,
{
secret: this.JWT_REFRESH_SECRET,
expiresIn: this.JWT_REFRESH_EXPIRES_IN,
} as any,
),
Copy link

Copilot AI Jan 11, 2026

Choose a reason for hiding this comment

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

The cast payload as object is unnecessary since TokenPayload is already an object type with defined properties. This cast reduces type safety without providing benefit. Similarly, casting the options object to any masks potential type issues with the JWT service configuration. Consider reviewing the JWT service types to ensure proper type compatibility rather than using type assertions.

Copilot uses AI. Check for mistakes.
]);

return {
Expand Down
4 changes: 2 additions & 2 deletions apps/backend/src/auth/strategies/JWT.strategy.spec.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -33,7 +33,7 @@ describe('JwtStrategy', () => {
it('should throw an error if JWT_SECRET is not set', () => {
jest.spyOn(configService, 'getOrThrow').mockReturnValue(null);

expect(() => new JwtStrategy(configService)).toThrowError(
expect(() => new JwtStrategy(configService)).toThrow(
'JwtStrategy requires a secret or key',
);
});
Expand Down Expand Up @@ -84,7 +84,7 @@ describe('JwtStrategy', () => {

const payload = { userId: 'test-user-id' };

expect(() => jwtStrategy.validate(req, payload)).toThrowError(
expect(() => jwtStrategy.validate(req, payload)).toThrow(
'No refresh token',
);
});
Expand Down
2 changes: 1 addition & 1 deletion apps/backend/src/auth/strategies/discord.strategy/discord.strategy.spec.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -43,7 +43,7 @@ describe('DiscordStrategy', () => {
it('should throw an error if Discord config is missing', () => {
jest.spyOn(configService, 'getOrThrow').mockReturnValueOnce(null);

expect(() => new DiscordStrategy(configService)).toThrowError(
expect(() => new DiscordStrategy(configService)).toThrow(
'OAuth2Strategy requires a clientID option',
);
});
Expand Down
2 changes: 1 addition & 1 deletion apps/backend/src/auth/strategies/discord.strategy/index.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -27,7 +27,7 @@ export class DiscordStrategy extends PassportStrategy(strategy, 'discord') {
callbackUrl: `${SERVER_URL}/v1/auth/discord/callback`,
scope: [DiscordPermissionScope.Email, DiscordPermissionScope.Identify],
fetchScope: true,
prompt: 'none',
prompt: 'none' as const,
};

super(config);
Expand Down
2 changes: 1 addition & 1 deletion apps/backend/src/auth/strategies/github.strategy.spec.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -43,7 +43,7 @@ describe('GithubStrategy', () => {
it('should throw an error if GitHub config is missing', () => {
jest.spyOn(configService, 'getOrThrow').mockReturnValueOnce(null);

expect(() => new GithubStrategy(configService)).toThrowError(
expect(() => new GithubStrategy(configService)).toThrow(
'OAuth2Strategy requires a clientID option',
);
});
Expand Down
4 changes: 2 additions & 2 deletions apps/backend/src/auth/strategies/github.strategy.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -22,10 +22,10 @@ export class GithubStrategy extends PassportStrategy(strategy, 'github') {
super({
clientID: GITHUB_CLIENT_ID,
clientSecret: GITHUB_CLIENT_SECRET,
redirect_uri: `${SERVER_URL}/v1/auth/github/callback`,
callbackURL: `${SERVER_URL}/v1/auth/github/callback`,
scope: 'user:read,user:email',
state: false,
});
} as any);
Copy link

Copilot AI Jan 11, 2026

Choose a reason for hiding this comment

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

The change from redirect_uri to callbackURL is correct and fixes a typo in the configuration property name. However, casting the entire configuration object to any masks other potential type issues. Consider checking the passport-github types to ensure all configuration properties are correctly typed rather than bypassing type checking with as any.

Copilot uses AI. Check for mistakes.
}

async validate(accessToken: string, refreshToken: string, profile: any) {
Expand Down
2 changes: 1 addition & 1 deletion apps/backend/src/auth/strategies/google.strategy.spec.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -44,7 +44,7 @@ describe('GoogleStrategy', () => {
it('should throw an error if Google config is missing', () => {
jest.spyOn(configService, 'getOrThrow').mockReturnValueOnce(null);

expect(() => new GoogleStrategy(configService)).toThrowError(
expect(() => new GoogleStrategy(configService)).toThrow(
'OAuth2Strategy requires a clientID option',
);
});
Expand Down
2 changes: 1 addition & 1 deletion apps/backend/src/lib/GetRequestUser.spec.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -28,7 +28,7 @@ describe('validateUser', () => {
});

it('should throw an error if the user does not exist', () => {
expect(() => validateUser(null)).toThrowError(
expect(() => validateUser(null)).toThrow(
new HttpException(
{
error: {
Expand Down
24 changes: 12 additions & 12 deletions apps/backend/src/song/song.controller.spec.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -258,7 +258,7 @@ describe('SongController', () => {
const query: SongListQueryDTO = { page: 1, limit: 10 };
const songList: SongPreviewDto[] = Array(10)
.fill(null)
.map((_, i) => ({ id: `song-${i}` } as SongPreviewDto));
.map((_, i) => ({ id: `song-${i}` } as unknown as SongPreviewDto));

mockSongService.querySongs.mockResolvedValueOnce({
content: songList,
Expand All @@ -280,7 +280,7 @@ describe('SongController', () => {
const query: SongListQueryDTO = { page: 1, limit: 10 };
const songList: SongPreviewDto[] = Array(5)
.fill(null)
.map((_, i) => ({ id: `song-${i}` } as SongPreviewDto));
.map((_, i) => ({ id: `song-${i}` } as unknown as SongPreviewDto));

mockSongService.querySongs.mockResolvedValueOnce({
content: songList,
Expand All @@ -300,7 +300,7 @@ describe('SongController', () => {
const query: SongListQueryDTO = { page: 3, limit: 10 };
const songList: SongPreviewDto[] = Array(10)
.fill(null)
.map((_, i) => ({ id: `song-${20 + i}` } as SongPreviewDto));
.map((_, i) => ({ id: `song-${20 + i}` } as unknown as SongPreviewDto));

mockSongService.querySongs.mockResolvedValueOnce({
content: songList,
Expand All @@ -321,7 +321,7 @@ describe('SongController', () => {
const query: SongListQueryDTO = { page: 1, limit: 10, q: 'test search' };
const songList: SongPreviewDto[] = Array(8)
.fill(null)
.map((_, i) => ({ id: `song-${i}` } as SongPreviewDto));
.map((_, i) => ({ id: `song-${i}` } as unknown as SongPreviewDto));

mockSongService.querySongs.mockResolvedValueOnce({
content: songList,
Expand All @@ -346,7 +346,7 @@ describe('SongController', () => {
};
const songList: SongPreviewDto[] = Array(3)
.fill(null)
.map((_, i) => ({ id: `rock-song-${i}` } as SongPreviewDto));
.map((_, i) => ({ id: `rock-song-${i}` } as unknown as SongPreviewDto));

mockSongService.querySongs.mockResolvedValueOnce({
content: songList,
Expand Down Expand Up @@ -430,7 +430,7 @@ describe('SongController', () => {
const q = 'test query';
const songList: SongPreviewDto[] = Array(5)
.fill(null)
.map((_, i) => ({ id: `song-${i}` } as SongPreviewDto));
.map((_, i) => ({ id: `song-${i}` } as unknown as SongPreviewDto));

mockSongService.querySongs.mockResolvedValueOnce({
content: songList,
Expand Down Expand Up @@ -493,7 +493,7 @@ describe('SongController', () => {
const q = 'test search';
const songList: SongPreviewDto[] = Array(10)
.fill(null)
.map((_, i) => ({ id: `song-${10 + i}` } as SongPreviewDto));
.map((_, i) => ({ id: `song-${10 + i}` } as unknown as SongPreviewDto));

mockSongService.querySongs.mockResolvedValueOnce({
content: songList,
Expand All @@ -516,7 +516,7 @@ describe('SongController', () => {
const q = 'popular song';
const songList: SongPreviewDto[] = Array(50)
.fill(null)
.map((_, i) => ({ id: `song-${i}` } as SongPreviewDto));
.map((_, i) => ({ id: `song-${i}` } as unknown as SongPreviewDto));

mockSongService.querySongs.mockResolvedValueOnce({
content: songList,
Expand All @@ -538,7 +538,7 @@ describe('SongController', () => {
const q = 'search term';
const songList: SongPreviewDto[] = Array(3)
.fill(null)
.map((_, i) => ({ id: `song-${40 + i}` } as SongPreviewDto));
.map((_, i) => ({ id: `song-${40 + i}` } as unknown as SongPreviewDto));

mockSongService.querySongs.mockResolvedValueOnce({
content: songList,
Expand Down Expand Up @@ -596,7 +596,7 @@ describe('SongController', () => {
const q = 'test';
const songList: SongPreviewDto[] = Array(25)
.fill(null)
.map((_, i) => ({ id: `song-${i}` } as SongPreviewDto));
.map((_, i) => ({ id: `song-${i}` } as unknown as SongPreviewDto));

mockSongService.querySongs.mockResolvedValueOnce({
content: songList,
Expand All @@ -623,7 +623,7 @@ describe('SongController', () => {
const q = 'trending';
const songList: SongPreviewDto[] = Array(10)
.fill(null)
.map((_, i) => ({ id: `song-${i}` } as SongPreviewDto));
.map((_, i) => ({ id: `song-${i}` } as unknown as SongPreviewDto));

mockSongService.querySongs.mockResolvedValueOnce({
content: songList,
Expand All @@ -644,7 +644,7 @@ describe('SongController', () => {
const q = 'search';
const songList: SongPreviewDto[] = Array(20)
.fill(null)
.map((_, i) => ({ id: `song-${40 + i}` } as SongPreviewDto));
.map((_, i) => ({ id: `song-${40 + i}` } as unknown as SongPreviewDto));

mockSongService.querySongs.mockResolvedValueOnce({
content: songList,
Expand Down
4 changes: 3 additions & 1 deletion apps/backend/src/song/song.controller.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -132,7 +132,9 @@ export class SongController {
[SongSortType.NOTE_COUNT, 'stats.noteCount'],
]);

const sortField = sortFieldMap.get(query.sort) ?? 'createdAt';
const sortField = query.sort
? sortFieldMap.get(query.sort) ?? 'createdAt'
: 'createdAt';
Copy link

Copilot AI Jan 11, 2026

Choose a reason for hiding this comment

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

The conditional check query.sort ? sortFieldMap.get(query.sort) ?? 'createdAt' : 'createdAt' is redundant. The original code sortFieldMap.get(query.sort) ?? 'createdAt' already handles the case where query.sort is undefined or null correctly, since Map.get(undefined) returns undefined and the nullish coalescing operator (??) will fall back to 'createdAt'. The added ternary check doesn't add value and makes the code more verbose.

Suggested change
const sortField = query.sort
? sortFieldMap.get(query.sort) ?? 'createdAt'
: 'createdAt';
const sortField = sortFieldMap.get(query.sort) ?? 'createdAt';

Copilot uses AI. Check for mistakes.
const isDescending = query.order ? query.order === 'desc' : true;

// Build PageQueryDTO with the sort field
Expand Down
6 changes: 2 additions & 4 deletions apps/backend/src/song/song.service.spec.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1225,8 +1225,7 @@ describe('SongService', () => {
exec: jest.fn().mockResolvedValue(songList),
};

mockSongModel.aggregate.mockReturnValue(mockAggregate as any);
mockSongModel.populate.mockResolvedValue(songList);
jest.spyOn(songModel, 'aggregate').mockReturnValue(mockAggregate as any);
Copy link

Copilot AI Jan 11, 2026

Choose a reason for hiding this comment

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

The test setup now uses jest.spyOn(songModel, 'aggregate') but the assertions still reference mockSongModel.aggregate and mockSongModel.populate. Since songModel is injected from the mocked mockSongModel, the assertions should be updated to use songModel instead of mockSongModel for consistency, or the spy should be on mockSongModel.

Copilot uses AI. Check for mistakes.

const result = await service.getRandomSongs(count);

Expand All @@ -1253,8 +1252,7 @@ describe('SongService', () => {
exec: jest.fn().mockResolvedValue(songList),
};

mockSongModel.aggregate.mockReturnValue(mockAggregate as any);
mockSongModel.populate.mockResolvedValue(songList);
jest.spyOn(songModel, 'aggregate').mockReturnValue(mockAggregate as any);
Copy link

Copilot AI Jan 11, 2026

Choose a reason for hiding this comment

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

Similar to the previous test, the setup uses jest.spyOn(songModel, 'aggregate') but the assertions reference mockSongModel.aggregate and mockSongModel.populate. The assertions should be updated to use songModel for consistency with the spy setup.

Copilot uses AI. Check for mistakes.

const result = await service.getRandomSongs(count, category);

Expand Down
14 changes: 14 additions & 0 deletions apps/backend/src/types/thumbnail.d.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,14 @@
declare module '@nbw/thumbnail/node' {
interface DrawParams {
notes: unknown;
startTick: number;
startLayer: number;
zoomLevel: number;
backgroundColor: string;
imgWidth: number;
imgHeight: number;
}

export function drawToImage(params: DrawParams): Promise<Buffer>;
export function drawNotesOffscreen(params: DrawParams): Promise<unknown>;
}
2 changes: 2 additions & 0 deletions apps/backend/tsconfig.json
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,8 @@
"allowSyntheticDefaultImports": true,
"sourceMap": true,
"emitDecoratorMetadata": true,
"moduleResolution": "node",
"noEmit": true,

// Path mapping
"baseUrl": ".",
Expand Down
23 changes: 11 additions & 12 deletions bun.lock
View file
Open in desktop

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

Loading