fix(formController): remove scope reference when form is destroyed #9315
Conversation
|
how are you going to call it a regression? the test fails with the parent commit of 729c238 as well |
|
fails in rc1 as well |
|
anyways, I think if there is a regression, you aren't testing what you're wanting to test. I'd be interested in seeing the specific code in your app where you ran into this, because you're probably running into an issue when you don't actually have a null parent form (in the test case you do, and that wouldn't have done what you expect before or after 729c238) |
|
hmm, nope, I can't get the "regression" to show itself in any fashion. Regardless, if you think this change is really necessary, we can certainly look at including it. I think you should make a slight change to the test, though. I'll leave a comment on the diff |
| @@ -58,6 +58,19 @@ describe('form', function() { | |||
| expect(form.alias).toBeUndefined(); | |||
| }); | |||
|
|
|||
| it('should remove form reference from the scope when form is removed from the DOM', function() { | |||
There was a problem hiding this comment.
make a note that you're only testing forms with no parent form, because this has always worked if they do have a named parent
|
Please explain why you think this is a regression, and what you were doing that has functionally changed. Ping me when you've done that |
|
You are right that this is not a regression, I just went over your commit and it seemed to me like this behavior has changed, but I was just confused. Anyway, I think this still needs to be fixed since currently we only remove the reference for nested forms and not for root forms. I'll fix your comments tonight. |
jeffbcross
force-pushed
the
master
branch
3 times, most recently
from
8292c21 to
7b9fddf
Compare
|
lgtm. thanks! |
No description provided.