Skip to content
This repository was archived by the owner on Sep 5, 2024. It is now read-only.

fix(chips): safety check before getting length - #6175

Closed
StefanFeederle wants to merge 1 commit into
angular:masterfrom
StefanFeederle:patch-1
Closed

fix(chips): safety check before getting length#6175
StefanFeederle wants to merge 1 commit into
angular:masterfrom
StefanFeederle:patch-1

Conversation

@StefanFeederle

Copy link
Copy Markdown
Contributor

If the model is not yet filled (ajax?) this functions fails with:
TypeError: Cannot read property 'length' of undefined

If we check the availability of "this.items" before accessing it we bypass this problem.

If the model is not yet filled (ajax?) this functions fails with:
TypeError: Cannot read property 'length' of undefined
Ported the fix from: sime/ui-select@1edc16b
@googlebot

Copy link
Copy Markdown

Thanks for your pull request. It looks like this may be your first contribution to a Google open source project. Before we can look at your pull request, you'll need to sign a Contributor License Agreement (CLA).

📝 Please visit https://cla.developers.google.com/ to sign.

Once you've signed, please reply here (e.g. I signed it!) and we'll verify. Thanks.


  • If you've already signed a CLA, it's possible we don't have your GitHub username or you're using a different email address. Check your existing CLA data and verify that your email is set on your git commits.
  • If you signed the CLA as a corporation, please let us know the company's name.

@topherfangiotopherfangio added needs: work needs: unit tests This PR needs unit tests to cover the changes being proposed labels Feb 4, 2016
@topherfangiotopherfangio added this to the 1.0.5 milestone Feb 4, 2016
@topherfangio

Copy link
Copy Markdown
Contributor

@StefanFeederle Can you please add a quick unit test which demonstrates the issue. I'd like to make sure we have a test so we don't introduce a regression later.

Thanks!

@ThomasBurlesonThomasBurleson modified the milestones: 1.0.5, 1.0.6Feb 4, 2016
@StefanFeederle

Copy link
Copy Markdown
ContributorAuthor

Hey @topherfangio, here are two codepens to highlight this issue. I don't know how to write unit tests, sorry.

error example
fixed example

The only difference is that i added the code of this PR in the fixed one.

If databinding to a not existing object {{ctrl.form.name}} does not throw errors, md-chips should behave the same and safety-check the object. I know this is not the best coding style and could be circumvented in my code, but i think its also important that angular-material is inherently consistent. This error is really hard to track down for newbies like me.

@StefanFeederle

Copy link
Copy Markdown
ContributorAuthor

I signed it!

@googlebot

Copy link
Copy Markdown

CLAs look good, thanks!

1 similar comment
@googlebot

Copy link
Copy Markdown

CLAs look good, thanks!

@ThomasBurlesonThomasBurleson modified the milestones: 1.0.6, 1.0.8Apr 4, 2016
@topherfangio

Copy link
Copy Markdown
Contributor

@ThomasBurleson Would love a unit test to go along with this, but is a NPE check, so it's super-simple. Think we should go ahead and just merge it?

@ThomasBurlesonThomasBurleson added pr: merge ready This PR is ready for a caretaker to review and removed needs: unit tests This PR needs unit tests to cover the changes being proposed labels Apr 19, 2016
@ThomasBurlesonThomasBurleson modified the milestones: 1.1.0, 1.0.8Apr 19, 2016
Sign up for freeto subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

pr: merge readyThis PR is ready for a caretaker to review

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants

@StefanFeederle@googlebot@topherfangio@ThomasBurleson