Skip to content
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

Remove util type checks #3135

Merged

Conversation

guilhermemntt
Copy link
Contributor

Closes #3131.

Copy link
Member

@amass01 amass01 left a comment

Choose a reason for hiding this comment

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

Thanks for this PR, left an inline suggestion.

@@ -391,7 +390,7 @@ export const exportStatToCSV = (opts) => (dispatch, getState) => {
// called once for each data line
const progressFunction = (time, series) => {
const values = allSeries.map((s) =>
!isUndefined(series[s.name]) ? s.valueFormatFunc(series[s.name]) : null
series[s.name] !== undefined ? s.valueFormatFunc(series[s.name]) : null
Copy link
Member

@amass01 amass01 Jan 9, 2021

Choose a reason for hiding this comment

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

This isn't enough, you could also check if typeof x === "undefined" but I would rather use lodash isUndefined func for that.

Lodash may have it's own version of isNumber, isNullOrUndefined funcs as well, checkout their documentation.

@guilhermemntt guilhermemntt force-pushed the guilhermemntt-remove-util-type-checks branch from 133af32 to 76ed888 Compare January 10, 2021 19:00
@guilhermemntt
Copy link
Contributor Author

guilhermemntt commented Jan 10, 2021

Thanks @amassarwi, you're totally right. Just to be clearer: something === undefined throws error if something hasn't been defined yet. So, there was a trap in @deprecated annotation and its suggestion. Following your suggestion now, lodash has all these type check, including isNil which is equivalent to isNullOrUndefined.

Copy link
Member

@amass01 amass01 left a comment

Choose a reason for hiding this comment

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

Noice 👍

@vctt94
Copy link
Member

vctt94 commented Jan 19, 2021

needs rebase

@guilhermemntt guilhermemntt force-pushed the guilhermemntt-remove-util-type-checks branch from 76ed888 to fcbdd88 Compare January 19, 2021 14:22
@guilhermemntt
Copy link
Contributor Author

Rebased.

@alexlyp
Copy link
Member

alexlyp commented Apr 20, 2021

Quite a few conflicts that will need to be dealt with.

@guilhermemntt guilhermemntt force-pushed the guilhermemntt-remove-util-type-checks branch from fcbdd88 to 6ad7791 Compare May 3, 2021 19:15
Copy link
Member

@matheusd matheusd left a comment

Choose a reason for hiding this comment

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

Nice job. Thanks for this!

@alexlyp alexlyp merged commit 703fd98 into decred:master May 3, 2021
alexlyp pushed a commit that referenced this pull request May 3, 2021
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.

Deprecated type checks
5 participants