feature/rework to typescript remove deprecated strict option #213
Conversation
TheDadi
commented
Oct 19, 2022
TheDadi
commented
- reworked the module to typescript
- removed the deprecated strict option
- added zod for options validation
|
At first glance, this looks great! I will review this weekend. |
| app.use( | ||
| koaBody({ | ||
| parsedMethods: [ | ||
| HttpMethodEnum.PATCH, |
There was a problem hiding this comment.
We'll need to note the use of this in the README
There was a problem hiding this comment.
This is just the typescript approach, you can still pass in 'GET', 'POST', 'PUT' as normal strings
There was a problem hiding this comment.
but we can note them if you want. You decide 🥇
src/utils/throwable-to-error.ts
Outdated
| return e; | ||
| } | ||
|
|
||
| const error = new Error(typeof e === 'object' ? `${JSON.stringify(e)}` : `${String(e)}`); |
There was a problem hiding this comment.
Can we simplify this to:
const error = new Error(typeof e === 'object' ? JSON.stringify(e) : '' + e);
README.md
Outdated
| @@ -141,22 +147,28 @@ Request Body: {"declaration":{"attributes":{"version":"1.0"}},"elements":[{"type | |||
| - `includeUnparsed` **{Boolean}** Toggles co-body returnRawBody option; if set to true, for form encoded and JSON requests the raw, unparsed request body will be attached to `ctx.request.body` using a `Symbol` ([see details](#a-note-about-unparsed-request-bodies)), default `false` | |||
| - `formidable` **{Object}** Options to pass to the formidable multipart parser | |||
| - `onError` **{Function}** Custom error handle, if throw an error, you can customize the response - onError(error, context), default will throw | |||
| - `strict` **{Boolean}** ***DEPRECATED*** If enabled, don't parse GET, HEAD, DELETE requests, default `true` | |||
| - `strict` **{Boolean}** **_DEPRECATED_** If enabled, don't parse GET, HEAD, DELETE requests, default `true` | |||
There was a problem hiding this comment.
I think we can delete documentation of this option since it's being removed
| @@ -1,29 +1,36 @@ | |||
| ## Problem/Feature Request Summary | |||
|
|
|||
There was a problem hiding this comment.
what tool did you use to format md files?
There was a problem hiding this comment.
i introduced prettier that also works for md files
| "@types/co-body": "^6.1.0", | ||
| "@types/formidable": "^2.0.5", | ||
| "@types/koa": "^2.13.5", | ||
| "co-body": "^6.1.0", |
There was a problem hiding this comment.
thanks for updating these :)
| @@ -2,10 +2,21 @@ | |||
| "name": "koa-body", | |||
| "version": "5.0.0", | |||
| "description": "A Koa body parser middleware. Supports multipart, urlencoded and JSON request bodies.", | |||
| "main": "index.js", | |||
| "types": "./index.d.ts", | |||
There was a problem hiding this comment.
we don't need to specify types?
There was a problem hiding this comment.
since we are a typescript project now, we export typings along side with our files and therefore typescript can resolve them without mentioning them in the package.json
There was a problem hiding this comment.
the types keyword would be there so non-typescript projects' IDEs could know where to find type info
I'll test this out locally
There was a problem hiding this comment.
@TheDadi yeah let's re-add the types property please
https://www.typescriptlang.org/docs/handbook/declaration-files/publishing.html#including-declarations-in-your-npm-package
| onFileBegin?: (name: string, file: File) => void; | ||
| }; | ||
|
|
||
| export const KoaBodyMiddlewareOptionsSchema = z.object({ |
| "scripts": { | ||
| "test": "mocha test/unit/", | ||
| "build": "npm run check-format && npm run test && npm run clean && npm run build:typescript", | ||
| "build:typescript": "tsc --build tsconfig.build.json", |
There was a problem hiding this comment.
Can we add "prepack": "npm run build", to ensure we only publish with built and tested code?
adding note about HttpMethodEnum
|
I think after the last 2 or so requested changes, this is good to merge! |
… feature/typescript-rewrite
|
@MarkHerhold should be ready now. 🍾 |
