Skip to content
This repository has been archived by the owner on Jul 30, 2018. It is now read-only.

Test and fix https://bugs.dojotoolkit.org/ticket/18637 #51

Closed
kfranqueiro opened this issue Jul 9, 2015 · 2 comments
Closed

Test and fix https://bugs.dojotoolkit.org/ticket/18637 #51

kfranqueiro opened this issue Jul 9, 2015 · 2 comments
Assignees
Labels
Milestone

Comments

@kfranqueiro
Copy link
Member

Given that IINM a lot of the aspect code in Dojo 2 was brought straight over from Dojo 1, I suspect it has the same issue here. We should port over this fix if the problem exists.

I'd also ideally like to make the aspect code a lot easier to follow in Dojo 2, but that will likely be another future ticket.

@kfranqueiro
Copy link
Member Author

I've confirmed locally with new tests that the same problem exists in dojo-core. In the process I also discovered that our typings for before did not permit for advising functions to not return anything, and we were clearly not testing that case either (since compilation would've failed if we were). I'll be addressing both of these issues at once.

WIP on my aspect-remove branch.

@kfranqueiro kfranqueiro self-assigned this Jul 9, 2015
@kfranqueiro kfranqueiro added the bug label Jul 9, 2015
@kfranqueiro
Copy link
Member Author

dojo-core's aspect actually had a different issue/failure with the same tests as the Dojo 1 issue. It was not nullifying advised.advice, which left no indication of "dead" advice handles, and actually caused removed advice functions to still be called in some cases.

This has been fixed in 22db350. (I forgot to update my commit message to reference this issue before merging, whoops.)

@dylans dylans added this to the Milestone 1 milestone Oct 27, 2017
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
Projects
None yet
Development

No branches or pull requests

2 participants