-
Notifications
You must be signed in to change notification settings - Fork 6
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: add asset metadata on select #222
Conversation
The latest updates on your projects. Learn more about Vercel for Git ↗︎
|
export const stringifyJsonFields = ( | ||
object: Record<string, any>, | ||
fields: string[] = [], | ||
): Record<string, any> => { | ||
const modifiedObject = { ...object }; | ||
|
||
for (const field of fields) { | ||
const value = _.get(object, field); | ||
const newValue = JSON.stringify(value, replaceNullWithEmptyString); | ||
_.set(modifiedObject, field, newValue); | ||
} | ||
|
||
return modifiedObject; | ||
}; |
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.
suggestion (non-blocking): since modifiedObject is being used as an accumulator, we could simplify this with a reduce:
export const stringifyJsonFields = ( | |
object: Record<string, any>, | |
fields: string[] = [], | |
): Record<string, any> => { | |
const modifiedObject = { ...object }; | |
for (const field of fields) { | |
const value = _.get(object, field); | |
const newValue = JSON.stringify(value, replaceNullWithEmptyString); | |
_.set(modifiedObject, field, newValue); | |
} | |
return modifiedObject; | |
}; | |
export const stringifyJsonFields = ( | |
object: Record<string, any>, | |
fields: string[] = [] | |
): Record<string, any> => { | |
return fields.reduce((acc, field) => { | |
const value = _.get(object, field); | |
const newValue = JSON.stringify(value, replaceNullWithEmptyString); | |
return _.set({ ...acc }, field, newValue); | |
}, object); | |
}; |
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.
question:
Wouldn't that modify my input object
? I'm being careful to note mutate the original object with this function.
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.
hmmm yeah since it's a reference, you might be messing with the OG, good point
export const replaceNullWithEmptyString = (_: any, value: any) => | ||
value === null ? '' : value; |
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.
question (blocking): I don't think this function needs two arguments, look like only value
is being used.
export const replaceNullWithEmptyString = (_: any, value: any) => | |
value === null ? '' : value; | |
suggestion (blocking): export const replaceNullWithEmptyString = (value: any) => | |
value === null ? '' : value; |
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.
comment:
The JSON.stringify()
function accepts a replacer
array or function.
JSON.stringify(value, replacer)
The function has two arguments, key
and value
. We pass replaceNullWithEmptyString
as the replacer function to JSON.stringify.
We only need value
, so we use _
to signify we don't intend to need or use the first argument, key
.
This is really just to keep stringifyJsonFields
readable, but it can come in handy wherever else you use JSON.stringify
.
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.
TIL thanks
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.
LGTM 🚀
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.
Tested locally, was able to retrieve properties ⭐
c500c2c
to
48f8ba6
Compare
543c8f9
to
95dc1f2
Compare
95dc1f2
to
5c0f4d1
Compare
48f8ba6
to
35af9f4
Compare
Co-authored-by: marco <[email protected]>
35af9f4
to
d272b1a
Compare
Description
stringifyJSONFields
to a helper directory for re-useBefore
Some images did not have width and height information available once selected.
After
All images, after selection, will include metadata. Specifically, width and height information.
Steps to test
Checklist
BREAKING CHANGE
in the body of the commit.