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: Add support for renderable Arrays of strings #275

Merged
merged 2 commits into from
May 9, 2017
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
8 changes: 7 additions & 1 deletion src/Helmet.js
Original file line number Diff line number Diff line change
Expand Up @@ -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") {
Copy link
Contributor

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.

if (
nestedChildren &&
typeof nestedChildren !== "string" &&
(
!Array.isArray(nestedChildren) || nestedChildren.some(nestedChild => typeof nestedChild !== "string")
)
) {
throw new Error(`Helmet expects a string as a child of <${child.type}>. Did you forget to wrap your children in braces? ( <${child.type}>{\`\`}</${child.type}> ) Refer to our API for more information.`);
}
}
Expand Down
4 changes: 2 additions & 2 deletions src/HelmetUtils.js
Original file line number Diff line number Diff line change
Expand Up @@ -301,8 +301,8 @@ const handleClientStateChange = (newState) => {
};

const updateTitle = (title, attributes) => {
if (typeof title === "string" && document.title !== title) {
document.title = title;
if (typeof title !== "undefined" && document.title !== title) {
document.title = Array.isArray(title) ? title.join("") : title;
}

updateAttributes(TAG_NAMES.TITLE, attributes);
Expand Down
17 changes: 17 additions & 0 deletions test/HelmetDeclarativeTest.js
Original file line number Diff line number Diff line change
Expand Up @@ -46,6 +46,23 @@ describe("Helmet - Declarative API", () => {
});
});

it("updates page title and allows children containing expressions", (done) => {
const someValue = "Some Great Title";
Copy link
Contributor

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?


ReactDOM.render(
<Helmet>
<title>Title: {someValue}</title>
</Helmet>,
container
);

requestIdleCallback(() => {
expect(document.title).to.equal("Title: Some Great Title");

done();
});
});

it("updates page title with multiple children", (done) => {
ReactDOM.render(
<div>
Expand Down