Skip to content
This repository was archived by the owner on Dec 15, 2022. It is now read-only.

Conversation

@lee-dohm
Copy link
Contributor

@lee-dohm lee-dohm commented Jan 9, 2017

Description of the Change

Protects against an undefined command target in the CommandLogger.

Alternate Designs

  • Considered fixing bug that was causing target to be undefined but figured that this code should be more flexible than that

Benefits

Possible Drawbacks

  • Doesn't address how something is calling a command on an undefined target

Applicable Issues

Fixes #135

@lee-dohm
Copy link
Contributor Author

lee-dohm commented Jan 9, 2017

/cc @50Wliu since you pointed this out to me 😀

@winstliu
Copy link
Contributor

winstliu commented Jan 9, 2017

It looks like these get used further down the line, such as here. Maybe an alternate design would be to throw early when adding a command without a target?

@lee-dohm
Copy link
Contributor Author

lee-dohm commented Jan 9, 2017

Yeah, good point. I may go back to my original idea and just return if there isn't a target. I'm going to write some tests around all of this too.

UziTech added a commit to UziTech/notifications that referenced this pull request Sep 25, 2019
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Uncaught TypeError: Cannot read property 'nodeName' of undefined

3 participants