Skip to content
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

Merged
merged 2 commits into from Jun 29, 2020

Conversation

@kiaking
Copy link
Member

@kiaking kiaking commented Jun 29, 2020

close #1426

This PR adds warning when removing non existing module. Currently, it fails due to runtime not exist in undefined error. It's follow up PR for #1426.

It will only warn in development mode, so it will not introduce any braking changes.

@kiaking kiaking requested a review from ktsn Jun 29, 2020
@kiaking kiaking self-assigned this Jun 29, 2020
@@ -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)) {

This comment has been minimized.

@ktsn

ktsn Jun 29, 2020
Member

Can we store the return value of getChild in a variable so that we reuse it with the next if condition?

This comment has been minimized.

@kiaking

kiaking Jun 29, 2020
Author Member

Oh yeah good point! Thanks. I've done it, and rebased (to the latest dev) and pushed 👍

@kiaking kiaking force-pushed the warn-when-unregistering-non-existing-module branch from 626b1de to 4fae622 Jun 29, 2020
@ktsn
ktsn approved these changes Jun 29, 2020
Copy link
Member

@ktsn ktsn left a comment

LGTM 👍

@ktsn ktsn merged commit 7cec79d into dev Jun 29, 2020
6 checks passed
6 checks passed
ci/circleci: install Your tests passed on CircleCI!
Details
ci/circleci: lint-types Your tests passed on CircleCI!
Details
ci/circleci: test-e2e Your tests passed on CircleCI!
Details
ci/circleci: test-ssr Your tests passed on CircleCI!
Details
ci/circleci: test-unit Your tests passed on CircleCI!
Details
deploy/netlify Deploy preview ready!
Details
@ktsn ktsn deleted the warn-when-unregistering-non-existing-module branch Jun 29, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Linked issues

Successfully merging this pull request may close these issues.

None yet

2 participants