Skip to content

Remove unneeded serialization#1714

Open
asutula wants to merge 3 commits intomainfrom
asutula/remove-serialization
Open

Remove unneeded serialization#1714
asutula wants to merge 3 commits intomainfrom
asutula/remove-serialization

Conversation

@asutula
Copy link
Member

@asutula asutula commented Feb 4, 2026

oRPC handles serialization of Date, bigint, etc, so we don't have to. This was just bothering me, it doesn't change anything about the logic of functionality of things.

@asutula asutula requested review from Copilot and dtbuchholz February 4, 2026 18:32
@vercel
Copy link
Contributor

vercel bot commented Feb 4, 2026

The latest updates on your projects. Learn more about Vercel for GitHub.

Project Deployment Actions Updated (UTC)
comps Ready Ready Preview, Comment Feb 4, 2026 6:46pm

@github-actions
Copy link
Contributor

github-actions bot commented Feb 4, 2026

📊 Test Coverage Report

Package Lines Statements Functions Branches
apps/api 2.51% 2.51% 43.70% 51.74%
apps/comps 0.26% 0.26% 37.66% 39.70%
packages/conversions 100.00% 100.00% 100.00% 100.00%
packages/db 4.27% 4.27% 20.86% 37.11%
packages/rewards 100.00% 100.00% 100.00% 100.00%
packages/services 51.93% 51.93% 65.18% 79.20% (+0.04%)
packages/staking-contracts 100.00% 100.00% 100.00% 100.00%

Copy link
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull request overview

This PR removes unnecessary manual serialization of Date and bigint types from RPC endpoints, relying on oRPC's built-in serialization capabilities instead. This simplifies the codebase by eliminating manual .toISOString() conversions for dates and .toString() conversions for bigints, as well as removing type definitions for serialized formats.

Changes:

  • Updated type definitions to use native Date and bigint types instead of strings
  • Simplified RPC endpoint handlers by removing manual serialization and restructuring return values
  • Updated frontend hooks and components to work directly with native types

Reviewed changes

Copilot reviewed 9 out of 9 changed files in this pull request and generated 3 comments.

Show a summary per file
File Description
apps/comps/types/nfl.ts Changed NflPrediction.createdAt from string to Date type
apps/comps/rpc/router/nfl/get-predictions.ts Removed manual date serialization and object wrapping; now returns predictions array directly
apps/comps/rpc/router/nfl/get-game-info.ts Removed manual date serialization and object wrapping; now returns game object directly
apps/comps/rpc/router/airdrop/get-next-season-eligibility.ts Removed manual bigint/date serialization; uses spread operator to return eligibility data
apps/comps/rpc/router/admin/bonus-boost/revoke.ts Removed .toISOString() call on revokedAt date field
apps/comps/hooks/useNextSeasonEligibility.ts Removed manual type conversions and interface definitions; uses inferred types from router outputs
apps/comps/components/nfl/predictions-table.tsx Updated to access predictions array directly instead of through data.predictions
apps/comps/components/nfl/nfl-standings-table.tsx Updated to access predictions array directly instead of through data.predictions
apps/comps/components/nfl/brier-score-chart.tsx Updated to access predictions array directly instead of through data.predictions

createdAt: p.createdAt.toISOString(),
})),
};
return predictions;
Copy link

Copilot AI Feb 4, 2026

Choose a reason for hiding this comment

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

The return structure has changed from an object with a predictions property to returning the array directly. This is a breaking change that will cause existing E2E tests to fail. The tests in apps/comps/e2e/tests/nfl-competition.test.ts expect to access data.predictions (lines 682, 683, 899, 900, 914), but this change makes the array the top-level return value.

If this breaking change is intentional, the E2E tests need to be updated accordingly. If not, the return structure should be preserved as { predictions } for backward compatibility.

Suggested change
return predictions;
return { predictions };

Copilot uses AI. Check for mistakes.
winner: game.winner,
},
};
return game;
Copy link

Copilot AI Feb 4, 2026

Choose a reason for hiding this comment

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

The return structure has changed from an object with a game property to returning the game object directly. This is a breaking change that will cause existing E2E tests to fail. The tests in apps/comps/e2e/tests/nfl-competition.test.ts expect to access data.game (lines 664, 852, 853), but this change makes the game object the top-level return value.

If this breaking change is intentional, the E2E tests need to be updated accordingly. If not, the return structure should be preserved as { game } for backward compatibility.

Suggested change
return game;
return { game };

Copilot uses AI. Check for mistakes.
Copy link
Contributor

@vercel vercel bot left a comment

Choose a reason for hiding this comment

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

Additional Suggestions:

  1. E2E test expects .data.game.status but the RPC handler now returns the game object directly without a .game wrapper
  1. E2E tests incorrectly access predictions array through .predictions property when handler now returns array directly
Fix on Vercel

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants