-
Notifications
You must be signed in to change notification settings - Fork 19
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
Refactor handlers, remove deprecated exports, v1 #399
Conversation
|
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #399 +/- ##
==========================================
+ Coverage 73.04% 73.07% +0.02%
==========================================
Files 78 82 +4
Lines 3024 3034 +10
Branches 454 456 +2
==========================================
+ Hits 2209 2217 +8
- Misses 810 812 +2
Partials 5 5 ☔ View full report in Codecov by Sentry. |
entry: { | ||
const: "src/const.ts", | ||
types: "src/types.ts", | ||
urls: "src/urls.ts", | ||
headers: "src/headers.ts", | ||
"saleor-app": "src/saleor-app.ts", | ||
"verify-jwt": "src/verify-jwt.ts", | ||
"verify-signature": "src/verify-signature.ts", | ||
"APL/index": "src/APL/index.ts", | ||
"APL/redis/index": "src/APL/redis/index.ts", | ||
"APL/vercel-kv/index": "src/APL/vercel-kv/index.ts", | ||
"app-bridge/index": "src/app-bridge/index.ts", | ||
"app-bridge/next/index": "src/app-bridge/next/index.ts", | ||
"settings-manager/index": "src/settings-manager/index.ts", | ||
"handlers/shared/index": "src/handlers/shared/index.ts", | ||
|
||
// Deprecated | ||
"middleware/index": "src/middleware/index.ts", | ||
|
||
// Mapped exports | ||
"handlers/next/index": "src/handlers/platforms/next/index.ts", | ||
}, |
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.
What does mean this api change - from array to object? Should keys include /index
?
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.
Yes, otherwise we wouldn't be able to do mapped exports in tsup, see: https://tsup.egoist.dev/#multiple-entrypoints
So for example:
[
"src/headers.ts"
]
is the same as:
{
headers: "src/headers.ts"
}
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.
is file name still valid? it contains random things
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.
Nope, this should have been in a single file for all saleor webhook types, I combined them 👍🏻
shared
folderplatform
folder, leaving public imports unchanged@/
alias, updated tests to support itNote
There are multiple files visible as "changed" in
handlers/next
andhandlers/platform/next
, but this is wrong, these files were simply moved to a different folder