Typechecking fixes by tomast1337 · Pull Request #77 · OpenNBS/NoteBlockWorld

tomast1337 · 2026-01-11T17:35:25Z

No description provided.

…in auth and song modules

Copilot

Pull request overview

This PR addresses various TypeScript typechecking issues in the backend codebase by:

Upgrading Next.js from 16.0.8 to 16.0.10
Removing unnecessary @nbw/database dependency from @nbw/song package
Adding TypeScript configuration options for better type checking
Creating type declarations for the @nbw/thumbnail/node module
Fixing test assertions and mock configurations
Correcting OAuth strategy configurations

Changes:

Updated Next.js and related dependencies to version 16.0.10
Added moduleResolution: "node" and noEmit: true to backend tsconfig.json
Created type declaration file for @nbw/thumbnail/node module
Fixed test mocks to use jest.spyOn instead of direct mock manipulation
Changed toThrowError to toThrow in multiple test files
Fixed GitHub strategy configuration from redirect_uri to callbackURL
Added type assertions to resolve type incompatibilities

Reviewed changes

Copilot reviewed 13 out of 15 changed files in this pull request and generated 5 comments.

Show a summary per file

File	Description
bun.lock	Updated Next.js from 16.0.8 to 16.0.10 and removed @nbw/database dependency from @nbw/song
apps/backend/tsconfig.json	Added moduleResolution and noEmit compiler options
apps/backend/src/types/thumbnail.d.ts	Created type declarations for @nbw/thumbnail/node module
apps/backend/src/song/song.service.spec.ts	Changed mock setup to use jest.spyOn
apps/backend/src/song/song.controller.spec.ts	Added explicit type casting with `as unknown as`
apps/backend/src/song/song.controller.ts	Added conditional check for sortField
apps/backend/src/lib/GetRequestUser.spec.ts	Changed toThrowError to toThrow
apps/backend/src/auth/strategies/google.strategy.spec.ts	Changed toThrowError to toThrow
apps/backend/src/auth/strategies/github.strategy.ts	Fixed callbackURL property and added any cast
apps/backend/src/auth/strategies/github.strategy.spec.ts	Changed toThrowError to toThrow
apps/backend/src/auth/strategies/discord.strategy/index.ts	Added const assertion to prompt property
apps/backend/src/auth/strategies/discord.strategy/discord.strategy.spec.ts	Changed toThrowError to toThrow
apps/backend/src/auth/strategies/JWT.strategy.spec.ts	Changed toThrowError to toThrow
apps/backend/src/auth/auth.service.ts	Added type casts for JWT payload and options
apps/backend/src/auth/auth.service.spec.ts	Added explicit parameter type

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Copilot · 2026-01-11T17:52:49Z

apps/backend/src/song/song.controller.ts

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


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 · 2026-01-11T17:52:49Z

apps/backend/src/auth/auth.service.ts

+      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,
+      ),


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 · 2026-01-11T17:52:49Z

apps/backend/src/auth/strategies/github.strategy.ts

    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);


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 · 2026-01-11T17:52:50Z

apps/backend/src/song/song.service.spec.ts


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


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 · 2026-01-11T17:52:50Z

apps/backend/src/song/song.service.spec.ts


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


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.

This change stems from the fact that Nest.js now defaults to ``` "module": "nodenext", "moduleResolution": "nodenext" ``` in recent releases. See: nestjs/nest#15919 (comment) This change lets us support modern `exports` syntax such as `@nbw/thumbnail/node`, without requiring external type declarations or even causing any additional typing errors (which was the case with `moduleResolution: nodenext` due to its stricter syntax around exports and extensions). Type-checking works fine, builds work fine, the backend starts just fine. ✅

No longer needed after changing `moduleResolution` to `bundler` in the backend.

…t` param

…StringValue`

+ don't assume 60s if no value is provided

…dation

tomast1337 added 4 commits January 11, 2026 11:51

update dev script in package.json to include webpack

3fa1910

refactor: update TypeScript configurations and improve type handling …

c332eb5

…in auth and song modules

Merge branch 'develop' of github.com:OpenNBS/NoteBlockWorld into develop

71f73f1

chore(deps): update Next.js and related packages to version 16.0.10

57c969c

tomast1337 requested review from Bentroen and Copilot January 11, 2026 17:49

Copilot started reviewing on behalf of tomast1337 January 11, 2026 17:49 View session

Copilot AI reviewed Jan 11, 2026

View reviewed changes

Bentroen added 10 commits January 14, 2026 21:18

chore: add @nbw/thumbnail/node type defs on backend

ed39809

chore(types): remove type definitions for @nbw/thumbnail/node

58a90d8

No longer needed after changing `moduleResolution` to `bundler` in the backend.

refactor(types): remove redundant ternary operator in song query `sor…

7429301

…t` param

refactor(types): remove catch-all types for jwtService.signAsync call

c5a22d5

fix(config): parse duration env configs on startup and coerce to `ms.…

8e43122

…StringValue`

fix(config) use config.getOrThrow to simplify logic in AuthModule

bf1acef

+ don't assume 60s if no value is provided

fix(config): validator's defaultMessage not being shown on env vali…

98c472b

…dation

chore(types): add TODO for improving typing on GitHub login strategy

ba1d3b9

test(song): remove duplicate authorization tests

b25030b

Bentroen approved these changes Jan 16, 2026

View reviewed changes

Bentroen merged commit 35a1bda into develop Jan 16, 2026
3 checks passed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

Typechecking fixes#77

Typechecking fixes#77
Bentroen merged 14 commits intodevelopfrom
fix/typechecking-issues

tomast1337 commented Jan 11, 2026

Uh oh!

Copilot AI left a comment

Uh oh!

Copilot AI Jan 11, 2026

Uh oh!

Copilot AI Jan 11, 2026

Uh oh!

Copilot AI Jan 11, 2026

Uh oh!

Copilot AI Jan 11, 2026

Uh oh!

Copilot AI Jan 11, 2026

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

Uh oh!

Conversation

tomast1337 commented Jan 11, 2026

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Reviewed changes

Uh oh!

Copilot AI Jan 11, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Jan 11, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Jan 11, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Jan 11, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Jan 11, 2026

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants