chore(ui,ui-react-core): Add type guard utils #3291
Conversation
🦋 Changeset detectedLatest commit: 4069dba The changes in this PR will be included in the next version bump. This PR includes changesets to release 6 packages
Not sure what this means? Click here to learn what changesets are. Click here if you're a maintainer who wants to add another changeset to this PR |
There was a problem hiding this comment.
Thanks for doing this!
What do you think of moving these in to a top level utils folder and putting them in to their own files? Currently the helpers folder is coupled to connected component utils/helpers. IMO it would make sense to keep these in their own folder since they are more generic, and to leave helpers as is to keep a separation of concerns (in particular since the connected component helpers/utils are most likely not living in the UI package long term)
There was a problem hiding this comment.
LGTM! Tentative approval for @calebpollman's comment on TS strict not liking isUndefined
| @@ -11,7 +11,7 @@ export function areEmptyArrays<T>(...values: T[]): boolean { | |||
| } | |||
|
|
|||
| function isEmptyObject<T>(value: T): boolean { | |||
There was a problem hiding this comment.
Maybe for a future PR, should these functions like isEmptyObject be in the UI package so other frameworks can use it?
| @@ -0,0 +1,59 @@ | |||
| import { isObject, isString, isUndefined } from '../typeGuards'; | |||
|
|
|||
| describe('isObject', () => { | |||
Description of changes
This change adds the following type guard utils:
isObjectisStringisUndefinedin an effort to make some progress towards removing
lodashdependency.Notes:
isStringis equivalent to the lodash implementation as it is used by inAppMessagging inside ofhandleMessageActionutil to check the validity of the URL we receive from the message payload delivered by Pinpoint via Amplify JS, and we want to cover edge case as much as possible since we have no control over what is sent over.isObjectdiverges from lodash implementation based on internal usage [when checking is something is an object we also check it's not an array so excluding arrays from this util is a quality of life improvement, additionally the likelihood of functions occurring based on current usage at least is low].Issue #, if available
Description of how you validated changes
Checklist
yarn testpassessideEffectsfield updatedBy submitting this pull request, I confirm that my contribution is made under the terms of the Apache 2.0 license.