-
Notifications
You must be signed in to change notification settings - Fork 660
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: Add support for renderable Arrays of strings #275
Conversation
Codecov Report
@@ Coverage Diff @@
## master #275 +/- ##
==========================================
+ Coverage 98.91% 98.92% +<.01%
==========================================
Files 3 3
Lines 277 278 +1
==========================================
+ Hits 274 275 +1
Misses 3 3
Continue to review full report at Codecov.
|
@@ -46,6 +46,23 @@ describe("Helmet - Declarative API", () => { | |||
}); | |||
}); | |||
|
|||
it("updates page title and allows children containing expressions", (done) => { | |||
const someValue = "Some Great Title"; |
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.
Should we have a test for const someValue = null
too?
@@ -187,7 +187,13 @@ const Helmet = (Component) => class HelmetWrapper extends React.Component { | |||
return warn(`Only elements types ${VALID_TAG_NAMES.join(", ")} are allowed. Helmet does not support rendering <${child.type}> elements. Refer to our API for more information.`); | |||
} | |||
|
|||
if (nestedChildren && typeof nestedChildren !== "string") { |
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.
Suggestion. Externalize the test into a function. That would make the code more declarative and easier to read on first glance and communicate better intent of the code.
Fixes #272