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.

Sign up for GitHub

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

Jump to bottom

Don't warn about casing in SSR for non-HTML NS #10452

Merged
merged 1 commit into from
Aug 13, 2017
Merged

Don't warn about casing in SSR for non-HTML NS #10452

merged 1 commit into from
Aug 13, 2017

Conversation

sophiebits
Copy link
Contributor

Fixes #10415.

flarnie
flarnie approved these changes Aug 13, 2017
View reviewed changes
Copy link
Contributor

@flarnie flarnie left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for doing this! Makes sense to me.

src/renderers/dom/fiber/ReactDOMFiberComponent.js
@@ -343,6 +319,8 @@ var ReactDOMFiberComponent = {
}
if (namespaceURI === HTML_NAMESPACE) {
if (__DEV__) {
// Should this check be gated by parent namespace? Not sure we want to
// allow <SVG> or <mATH>.
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

nit - probably meant "<MATH>" not "<mATH>"

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is what I meant. :)

src/renderers/dom/shared/__tests__/ReactServerRendering-test.js
@@ -772,4 +772,36 @@ describe('ReactDOMServer', () => {
);
}).toThrowError(/Cannot assign to read only property.*/);
});

it('warns about lowercase html but not in svg tags', () => {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

awesome - thanks for adding a test!

@gaearon gaearon mentioned this pull request Aug 13, 2017
Closed
@sophiebits sophiebits merged commit 5e85f00 into facebook:master Aug 13, 2017
@bvaughn bvaughn mentioned this pull request Sep 7, 2017
Closed
@sebmarkbage sebmarkbage mentioned this pull request Mar 24, 2021
Merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@flarnie flarnie flarnie approved these changes

Assignees
No one assigned
Labels
CLA Signed
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
3 participants
@sophiebits @flarnie @facebook-github-bot