imagekit-developer / imagekit-angular

Angular SDK for ImageKit.io client side file upload and URL generation
https://imagekit.io
18 stars 20 forks source link

Sdk v2 upgrade #36

Closed ngyongzhen closed 1 year ago

ngyongzhen commented 1 year ago

Sdk v2 upgrade

lgtm-com[bot] commented 1 year ago

This pull request fixes 1 alert when merging 765a5be0edd547305c9ec8b0310abfc66c0cd110 into 141953ccb2220b88357cfdf94d2422d81320a4a5 - view on LGTM.com

fixed alerts:

Heads-up: LGTM.com's PR analysis will be disabled on the 5th of December, and LGTM.com will be shut down ⏻ completely on the 16th of December 2022. Please enable GitHub code scanning, which uses the same CodeQL engine :gear: that powers LGTM.com. For more information, please check out our post on the GitHub blog.

lgtm-com[bot] commented 1 year ago

This pull request fixes 1 alert when merging 8130a7f8d046c9ac01c8b6219a7100df86e72a7a into 141953ccb2220b88357cfdf94d2422d81320a4a5 - view on LGTM.com

fixed alerts:

Heads-up: LGTM.com's PR analysis will be disabled on the 5th of December, and LGTM.com will be shut down ⏻ completely on the 16th of December 2022. Please enable GitHub code scanning, which uses the same CodeQL engine :gear: that powers LGTM.com. For more information, please check out our post on the GitHub blog.

codecov[bot] commented 1 year ago

Codecov Report

:exclamation: No coverage uploaded for pull request base (master@141953c). Click here to learn what that means. Patch has no changes to coverable lines.

:exclamation: Current head b8020a8 differs from pull request most recent head af32dc9. Consider uploading reports for the commit af32dc9 to get more accurate results

Additional details and impacted files ```diff @@ Coverage Diff @@ ## master #36 +/- ## ========================================= Coverage ? 92.05% ========================================= Files ? 6 Lines ? 277 Branches ? 52 ========================================= Hits ? 255 Misses ? 4 Partials ? 18 ``` Help us with your feedback. Take ten seconds to tell us [how you rate us](https://about.codecov.io/nps?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=None). Have a feature suggestion? [Share it here.](https://app.codecov.io/gh/feedback/?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=None)

:umbrella: View full report at Codecov.
:loudspeaker: Do you have feedback about the report comment? Let us know in this issue.

lgtm-com[bot] commented 1 year ago

This pull request fixes 1 alert when merging b33ab3d1a547d1f2252cb58d287ff57acf5dab88 into 141953ccb2220b88357cfdf94d2422d81320a4a5 - view on LGTM.com

fixed alerts:

Heads-up: LGTM.com's PR analysis will be disabled on the 5th of December, and LGTM.com will be shut down ⏻ completely on the 16th of December 2022. Please enable GitHub code scanning, which uses the same CodeQL engine :gear: that powers LGTM.com. For more information, please check out our post on the GitHub blog.

lgtm-com[bot] commented 1 year ago

This pull request fixes 1 alert when merging c9c97d9713a124979f2df511a7e0994be0996554 into 141953ccb2220b88357cfdf94d2422d81320a4a5 - view on LGTM.com

fixed alerts:

Heads-up: LGTM.com's PR analysis will be disabled on the 5th of December, and LGTM.com will be shut down ⏻ completely on the 16th of December 2022. Please enable GitHub code scanning, which uses the same CodeQL engine :gear: that powers LGTM.com. For more information, please check out our post on the GitHub blog.

lgtm-com[bot] commented 1 year ago

This pull request fixes 1 alert when merging d4e8d84b1887eecbe5a929eba4259a081982439a into 141953ccb2220b88357cfdf94d2422d81320a4a5 - view on LGTM.com

fixed alerts:

Heads-up: LGTM.com's PR analysis will be disabled on the 5th of December, and LGTM.com will be shut down ⏻ completely on the 16th of December 2022. Please enable GitHub code scanning, which uses the same CodeQL engine :gear: that powers LGTM.com. For more information, please check out our post on the GitHub blog.

lgtm-com[bot] commented 1 year ago

This pull request fixes 1 alert when merging ec331f9a96bd1ddecf116995e8f70bed6ad4a6a0 into 141953ccb2220b88357cfdf94d2422d81320a4a5 - view on LGTM.com

fixed alerts:

Heads-up: LGTM.com's PR analysis will be disabled on the 5th of December, and LGTM.com will be shut down ⏻ completely on the 16th of December 2022. Please enable GitHub code scanning, which uses the same CodeQL engine :gear: that powers LGTM.com. For more information, please check out our post on the GitHub blog.

lgtm-com[bot] commented 1 year ago

This pull request fixes 1 alert when merging 9d86d09635a41241672793300ad0f3b866d21613 into 141953ccb2220b88357cfdf94d2422d81320a4a5 - view on LGTM.com

fixed alerts:

Heads-up: LGTM.com's PR analysis will be disabled on the 5th of December, and LGTM.com will be shut down ⏻ completely on the 16th of December 2022. Please enable GitHub code scanning, which uses the same CodeQL engine :gear: that powers LGTM.com. For more information, please check out our post on the GitHub blog.

lgtm-com[bot] commented 1 year ago

This pull request fixes 1 alert when merging f97154b2f79add965aa4dce931ef515f7e49e25f into 141953ccb2220b88357cfdf94d2422d81320a4a5 - view on LGTM.com

fixed alerts:

Heads-up: LGTM.com's PR analysis will be disabled on the 5th of December, and LGTM.com will be shut down ⏻ completely on the 16th of December 2022. Please enable GitHub code scanning, which uses the same CodeQL engine :gear: that powers LGTM.com. For more information, please check out our post on the GitHub blog.

lgtm-com[bot] commented 1 year ago

This pull request fixes 1 alert when merging 468b4eb87ff5a7e5fa5039fcdab37f82c0908fb3 into 141953ccb2220b88357cfdf94d2422d81320a4a5 - view on LGTM.com

fixed alerts:

Heads-up: LGTM.com's PR analysis will be disabled on the 5th of December, and LGTM.com will be shut down ⏻ completely on the 16th of December 2022. Please enable GitHub code scanning, which uses the same CodeQL engine :gear: that powers LGTM.com. For more information, please check out our post on the GitHub blog.

lgtm-com[bot] commented 1 year ago

This pull request fixes 1 alert when merging 0a5a7caeff29e40dc1595ac1f31233e94ba808e3 into 141953ccb2220b88357cfdf94d2422d81320a4a5 - view on LGTM.com

fixed alerts:

Heads-up: LGTM.com's PR analysis will be disabled on the 5th of December, and LGTM.com will be shut down ⏻ completely on the 16th of December 2022. Please enable GitHub code scanning, which uses the same CodeQL engine :gear: that powers LGTM.com. For more information, please check out our post on the GitHub blog.

lgtm-com[bot] commented 1 year ago

This pull request fixes 1 alert when merging b6224daec92d315d7f8aa9d412defc84dbd3f068 into 141953ccb2220b88357cfdf94d2422d81320a4a5 - view on LGTM.com

fixed alerts:

Heads-up: LGTM.com's PR analysis will be disabled on the 5th of December, and LGTM.com will be shut down ⏻ completely on the 16th of December 2022. Please enable GitHub code scanning, which uses the same CodeQL engine :gear: that powers LGTM.com. For more information, please check out our post on the GitHub blog.

lgtm-com[bot] commented 1 year ago

This pull request fixes 1 alert when merging 928386ac1bcd0ddf9107413aad06517fb0380892 into 141953ccb2220b88357cfdf94d2422d81320a4a5 - view on LGTM.com

fixed alerts:

Heads-up: LGTM.com's PR analysis will be disabled on the 5th of December, and LGTM.com will be shut down ⏻ completely on the 16th of December 2022. Please enable GitHub code scanning, which uses the same CodeQL engine :gear: that powers LGTM.com. For more information, please check out our post on the GitHub blog.

lgtm-com[bot] commented 1 year ago

This pull request fixes 1 alert when merging 8390675a1a019421896741f955296233e81ca912 into 141953ccb2220b88357cfdf94d2422d81320a4a5 - view on LGTM.com

fixed alerts:

Heads-up: LGTM.com's PR analysis will be disabled on the 5th of December, and LGTM.com will be shut down ⏻ completely on the 16th of December 2022. Please enable GitHub code scanning, which uses the same CodeQL engine :gear: that powers LGTM.com. For more information, please check out our post on the GitHub blog.

lgtm-com[bot] commented 1 year ago

This pull request fixes 1 alert when merging 0941865f297a91a42029bfdaee4fd501b7d97cfe into 141953ccb2220b88357cfdf94d2422d81320a4a5 - view on LGTM.com

fixed alerts:

Heads-up: LGTM.com's PR analysis will be disabled on the 5th of December, and LGTM.com will be shut down ⏻ completely on the 16th of December 2022. Please enable GitHub code scanning, which uses the same CodeQL engine :gear: that powers LGTM.com. For more information, please check out our post on the GitHub blog.

ngyongzhen commented 1 year ago

@manu4543

For Angular, the default implementation of ErrorHandler prints error messages to the console as described here: https://angular.io/api/core/ErrorHandler

This means that when IK component is not given the proper fields or values, the page still renders and component still gets rendered with the proper html tags i.e for ik-video component, the video tag would still be rendered.

For example, refer to screenshots taken for ik-video when path/src is not provided.

Console error

Screenshot 2022-12-18 at 12 14 13 AM

HTML tag rendering

Screenshot 2022-12-18 at 12 13 24 AM

HTML page loads

Screenshot 2022-12-18 at 12 12 27 AM

Please let me know if that suffices for the error-handling aspect.

imagekitio commented 1 year ago

@manu4543

For Angular, the default implementation of ErrorHandler prints error messages to the console as described here: https://angular.io/api/core/ErrorHandler

This means that when IK component is not given the proper fields or values, the page still renders and component still gets rendered with the proper html tags i.e for ik-video component, the video tag would still be rendered.

For example, refer to screenshots taken for ik-video when path/src is not provided.

Console error Screenshot 2022-12-18 at 12 14 13 AM

HTML tag rendering Screenshot 2022-12-18 at 12 13 24 AM

HTML page loads Screenshot 2022-12-18 at 12 12 27 AM

Please let me know if that suffices for the error-handling aspect.

Yes

ngyongzhen commented 1 year ago

Cypress tests are still not running as part of CI, can you show me where are the logs in action?

They are running now as part of push/PR activity.