fundamental-ngx
fundamental-ngx copied to clipboard
SAP
fix(core): type incorrect assumption of input always existing
Related Issue(s)
closes #12001
Description
Input might be undefined, we can't assume it's not. Added an example, if you type undefined to that input, on main it'll reproduce the error of the linked issue.
Type of filterFn is wrong on multi-input.component (this['dropdownValues']) as it contains a package private type. This is why in the example I used anys
Visit the preview URL for this PR (updated for commit 678c3ff):
https://fundamental-ngx-gh--pr12240-mishandled-optional-z3skwvql.web.app
(expires Fri, 13 Sep 2024 10:36:37 GMT)
🔥 via Firebase Hosting GitHub Action 🌎
Sign: 41b993ee8e451bd7c6770b342ce142dc886eacff
![github-actions[bot] avatar](https://avatars.githubusercontent.com/in/15368?v=4 "Published by a pub.dev verified publisher"){kind=small}
Deploy Preview for fundamental-ngx ready!
| Name | Link |
|---|---|
| Latest commit | 678c3ffa67bc4b5cc4ef9e3740211664c39a793d |
| Latest deploy log | https://app.netlify.com/sites/fundamental-ngx/deploys/66e0202a185fdc00080924c3 |
| Deploy Preview | https://deploy-preview-12240--fundamental-ngx.netlify.app |
| Preview on mobile | Toggle QR Code...Use your smartphone camera to open QR code link. |
To edit notification comments on pull requests, go to your Netlify site configuration.
![netlify[bot] avatar](https://avatars.githubusercontent.com/in/13473?v=4 "Published by a pub.dev verified publisher"){kind=small}
@droshev this change i suspect failed some e2e tests, so i tried to fix them as well
I've created a new PR for the e2e testing commit #12253 so you can revert it from this one and only keep the changes for the multi-input issue
@mikerodonnell89 I hope you see how it's difficult to create a stackblitz to an wrongly working internal function. nevertheless here you can find an example. type in 'undefined' and you'll get a console error. This example is partly incorrect, as filterFn would expect an array returning function, and does have a default value, but only through that can I reach (right now) the multiAnnouncerOptions input.
multiAnnouncerOptions does not have a default value and is not a required input, and as such we cannot assume it to exist. this will cause the console error above.
