Skip to content

Implements PheroUnchecked - #138

Open
kamilafsar wants to merge 5 commits into
developfrom
feature/phero-unchecked
Open

Implements PheroUnchecked#138
kamilafsar wants to merge 5 commits into
developfrom
feature/phero-unchecked

Conversation

@kamilafsar

@kamilafsar kamilafsar commented Sep 25, 2024

Copy link
Copy Markdown

With the PheroUnchecked utility type, a user can tag a context member to denote that Phero should skip validation for input/output of the middleware/function.

Last 2 commits are random improvements to the codebase

With the PheroUnchecked utility type, a user can tag a context member to denote that Phero should skip validation for input/output of the middleware/function.

@nloomans nloomans left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Nice work! Looking forward to use PheroUnchecked

Comment thread packages/core/src/parsePheroApp/parseServiceContextType.ts
Comment thread packages/server/src/index.ts
Comment thread packages/core/src/parsePheroApp/parseServiceContextType.ts
Comment thread packages/core/src/parsePheroApp/parseServiceContextType.ts

@nloomans nloomans left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Small issue with the documentation, but otherwise LGTM

Comment on lines +18 to +32
/**
* By default all middleware input is parsed by Phero. With PheroUnchecked<T> you can
* mark a property within your middleware context, so that Phero will skip it during
* the parsing step. E.g.
* @example
* ```typescript
* async function getArticle(ctx: PheroContext<{ db: PheroUnchecked<RealDB> }>): Promise<string> {
* return ctx.db.query()
* }
*
* async function myMiddleware(context: PheroContext, next: PheroNextFunction<{ db: PheroUnchecked<DBClient> }>) {
* await next({ db: new DBClient() })
* }
* ```
*/

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

"so that Phero will skip it during the parsing step" sounds like Phero will just omit the type. I also don't think users will understand what the "parsing step" refers to.

The example also has issues. JSDoc isn't markdown, so the ```typescript prefix should be omitted. Also the example shows incorrect usage, since we are providing DBClient but asking for RealDB.

How about this?

/**
 * PheroUnchecked<T> is a special type that tells Phero it should not check at
 * runtime if the types actually match what we expect. It may only be used
 * within {@link PheroContext}, and must be provided by a middleware. This is
 * useful for putting types in {@link PheroContext} that would normally be
 * unsupported, such as a classes.
 *
 * @example
 * async function getArticle(ctx: PheroContext<{ db: PheroUnchecked<DBClient> }>): Promise<string> {
 *   return ctx.db.query()
 * }
 *
 * async function myMiddleware(context: PheroContext, next: PheroNextFunction<{ db: PheroUnchecked<DBClient> }>) {
 *   await next({ db: new DBClient() })
 * }
 */

@kamilafsar kamilafsar Oct 9, 2024

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

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

remove the "at runtime" in case Phero will not complain if this does compile:

*   async function getArticle(ctx: PheroContext<{ db: PheroUnchecked<RealDB> }>): Promise<string> {
 *     return ctx.db.query()
 *   }
 *
 *   async function myMiddleware(context: PheroContext, next: PheroNextFunction<{ db: PheroUnchecked<DBClient> }>) {
 *     await next({ db: new DBClient() })
 *   }

Notice the RealDB/DBClient typo

Base automatically changed from feature/upgrade-tsc to develop October 18, 2024 13:15
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