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
easy fixes #1325
easy fixes #1325
Conversation
5fc714d
to
fb51fa9
Compare
@@ -167,8 +168,6 @@ export function hasPlaceholder(uri, path, storeOverride = store) { | |||
isCollapsibleList = isComponentList && !!_.get(schema, `${componentListProp}.collapse`), | |||
isComponentProp = schema && !!schema[componentProp]; | |||
|
|||
// if there isn't a placeholder at all, return negative quickly | |||
// since we're ususally checking a bunch of fields/groups | |||
if (!placeholder) { | |||
return false; | |||
} |
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.
no explanation needed for early returns because they're the best. It's so much nicer reserving your unindented code for your main logical branch and your indented code for early returns, error handling, etc.
_.each(placeholders[uri], (placeholder) => placeholder.$destroy()); | ||
placeholders[uri] = null; | ||
} | ||
placeholders[uri] && placeholders[uri].forEach((ph) => ph.$destroy()); |
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.
this doesn't remove / nullify placeholders[uri]
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.
I'm not quite sure what these are fixing, but (aside from the removal code) they seem fine
Depends on #1324