-
-
Notifications
You must be signed in to change notification settings - Fork 9.6k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
fix: warn when unregistering non existing module #1786
Conversation
src/module/module-collection.js
Outdated
@@ -49,7 +49,20 @@ export default class ModuleCollection { | |||
unregister (path) { | |||
const parent = this.get(path.slice(0, -1)) | |||
const key = path[path.length - 1] | |||
if (!parent.getChild(key).runtime) return | |||
|
|||
if (!parent.getChild(key)) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Can we store the return value of getChild
in a variable so that we reuse it with the next if condition?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Oh yeah good point! Thanks. I've done it, and rebased (to the latest dev) and pushed 👍
626b1de
to
4fae622
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM 👍
close #1426
This PR adds warning when removing non existing module. Currently, it fails due to
runtime
not exist inundefined
error. It's follow up PR for #1426.It will only warn in development mode, so it will not introduce any braking changes.