-
-
Notifications
You must be signed in to change notification settings - Fork 422
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.
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
feat: expose TRPC transformer from api package #1189
Open
ochicf
wants to merge
9
commits into
t3-oss:main
Choose a base branch
from
ochicf:feat/trpc-transformer
base: main
Could not load branches
Branch not found: {{ refName }}
Loading
Could not load tags
Nothing to show
Loading
Are you sure you want to change the base?
Some commits from the old base branch may be removed from the timeline,
and old review comments may become outdated.
Open
Changes from 8 commits
Commits
Show all changes
9 commits
Select commit
Hold shift + click to select a range
e9dc219
fix(deps): update dependency @shopify/flash-list to v1.7.1
renovate[bot] ce79802
feat: move transformer to a separate module and expose as `@acme/api/…
ochicf 049e67c
refactor: use `@acme/api/transformer` in nextjs app
ochicf 3ee7a40
chore: move `@acme/api` from dev to direct dependency
ochicf 525297e
refactor: use `@acme/api/transformer` in expo app
ochicf 4077f77
build: add `@acme/api/transformer` in tsconfig so it can be actually …
ochicf 499d957
chore: add eslint rule to prevent importing values from modules other…
ochicf 960f354
chore: remove `sperjson` as direct dependency from apps
ochicf 78b07c6
doc: add link to official tRPC docs
ochicf File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,22 @@ | ||
import type { TRPCCombinedDataTransformer } from "@trpc/server"; | ||
import SuperJSON from "superjson"; | ||
|
||
/** | ||
* tRPC transformer, used to serialize/deserialize data between client and server. | ||
* This export is used internally in `@acme/api` and in both `apps/nextjs` and `apps/expo`. | ||
* | ||
* ! IMPORTANT: changing this is a BREAKING CHANGE ! | ||
* ? Even though this helps swapping transformers, bear in mind that distributed Expo apps | ||
* ? will have the previous transformer bundled in their device, which will cause a mismatch | ||
* ? in how the data is sent and received (basically, all API requests will fail). | ||
*/ | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. good callout. add a |
||
export const transformer: TRPCCombinedDataTransformer = { | ||
input: { | ||
serialize: SuperJSON.serialize, | ||
deserialize: SuperJSON.deserialize, | ||
}, | ||
output: { | ||
serialize: SuperJSON.serialize, | ||
deserialize: SuperJSON.deserialize, | ||
}, | ||
}; |
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Oops, something went wrong.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Have Expo shipped tree shaking yet? If not I think including this as a prod dep will leak backend code
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
SDK 52 it looks like https://x.com/anis_RNCore/status/1841790356930887859
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Good point, didn't though of that. I'm not sure how to check if it's happening (I'm quite new with Expo and RN). Also, not sure if updating SDK is easy or not and should be done in this PR or in a different one.
Another option would be to move the transformer logic into a separate package so we are 100% sure it doesn't bring any BE code. For example
@acme/transformer
or@acme/api-transformer
(though IMHO it feels much better to have it as a module in@acme/api
).There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I am personally scared of adding an api package as a dependency, regardles of wether or not Expo does tree-shaking correctly or not. Might not be an issue, but it's something to be mindful of.
I personally have a
@acme/shared
package that exports common utils/stuff that is safe to be shared everywhere. Adding another package just for the tranformer export might be a bit verbose for this repo thoughThere was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Cool, yeah it makes sense. I'll prepare that and revert the api package to just dev dependency in Expo.