feat(middleware): support creation of standalone functions #229
Conversation
…ed `createMiddleware` function
…ntal_createMiddleware`
|
The latest updates on your projects. Learn more about Vercel for Git ↗︎
|
| const k = key as keyof typeof obj2; | ||
| // eslint-disable-next-line | ||
| if (typeof obj2[k] === "object" && Object.hasOwn(obj1, k)) { | ||
| // @ts-expect-error |
There was a problem hiding this comment.
Consider handling arrays explicitly. If obj2[k] is an array, it might be better to replace obj1[k] with a copy of obj2[k] instead of treating it as an object for merging.
There was a problem hiding this comment.
The check typeof obj2[k] === "object" will return true for null values, which could cause issues. Consider adding an explicit check to ensure obj2[k] is not null.
There was a problem hiding this comment.
Thank you for reviewing the code. Would it be better to use lodash merge function in your opinion?
There was a problem hiding this comment.
I definitely think using a 3rd-party library would be better, although I don't think lodash has been well-maintained recently? Maybe consider ramda?
There was a problem hiding this comment.
The thing is: ramda is published as a whole library, while lodash provides lodash.merge as a standalone package, which is obviously much smaller in size. deepmerge or deepmerge-ts also appear to be viable options. What do you think?
| @@ -2,6 +2,22 @@ export const DEFAULT_SERVER_ERROR_MESSAGE = "Something went wrong while executin | |||
|
|
|||
| export const isError = (error: unknown): error is Error => error instanceof Error; | |||
|
|
|||
| export const deepMerge = (obj1: object, obj2: object) => { | |||
| for (const key of Object.keys(obj2)) { | |||
There was a problem hiding this comment.
To prevent prototype pollution attacks, consider adding a check to skip merging keys like proto, constructor, and prototype.
| @@ -2,6 +2,22 @@ export const DEFAULT_SERVER_ERROR_MESSAGE = "Something went wrong while executin | |||
|
|
|||
| export const isError = (error: unknown): error is Error => error instanceof Error; | |||
|
|
|||
| export const deepMerge = (obj1: object, obj2: object) => { | |||
There was a problem hiding this comment.
Consider adding a validation to ensure that obj1 and obj2 are non-null objects. This will prevent issues when the function is called with non-object arguments.
|
If you use the tsup tree shaking option, it should package only the code
you end up using in the final build output. Lodasb certainly works, but the
code is about 5 years old. I don't have experience with the other
libraries.
…On Mon, Aug 12 2024 at 19:47, Edoardo Ranghieri ***@***.***> wrote:
***@***.**** commented on this pull request.
------------------------------
In packages/next-safe-action/src/utils.ts
<#229 (comment)>
:
> @@ -2,6 +2,22 @@ export const DEFAULT_SERVER_ERROR_MESSAGE = "Something went wrong while executin
export const isError = (error: unknown): error is Error => error instanceof Error;
+export const deepMerge = (obj1: object, obj2: object) => {
+ for (const key of Object.keys(obj2)) {
+ const k = key as keyof typeof obj2;
+ // eslint-disable-next-line
+ if (typeof obj2[k] === "object" && Object.hasOwn(obj1, k)) {
+ // @ts-expect-error
The thing is: ramda is published as a whole library, while lodash provides
lodash.merge <https://www.npmjs.com/package/lodash.merge> as a standalone
package, which is obviously much smaller in size. deepmerge
<https://www.npmjs.com/package/deepmerge> or deepmerge-ts
<https://www.npmjs.com/package/deepmerge-ts> also appear to be viable
options. What do you think?
—
Reply to this email directly, view it on GitHub
<#229 (comment)>,
or unsubscribe
<https://github.com/notifications/unsubscribe-auth/AAHDDRYBUBIRYVHK33U37Q3ZRFCQ5AVCNFSM6AAAAABMMT2J3GVHI2DSMVQWIX3LMV43YUDVNRWFEZLROVSXG5CSMV3GSZLXHMZDEMZUGE4TONZRHA>
.
You are receiving this because you commented.Message ID:
***@***.***>
|
|
@mcalhoun I chose to use deepmerge-ts, which seems to be an up to date and more performant variant of deepmerge. Apparently, if you install the package as |
|
🎉 This PR is included in version 7.6.0 🎉 The release is available on: Your semantic-release bot 📦🚀 |
Proposed changes
Code in this PR requires context to be an object, it extends it by default, and enables creation of standalone middleware functions via built-in
experimental_createMiddlewareutility.Related issue(s) or discussion(s)
re #222