-
Notifications
You must be signed in to change notification settings - Fork 47k
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
Update to Jest 22 #11956
Update to Jest 22 #11956
Changes from 13 commits
2ab6aeb
ac83604
91c30e2
c8e8795
fa5770f
6101acd
7956d72
5e739bc
9dc3a61
6680473
7cd3cf8
1ba6a45
ed93325
14805c7
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -29,7 +29,11 @@ function initModules() { | |
}; | ||
} | ||
|
||
const {resetModules, itRenders} = ReactDOMServerIntegrationUtils(initModules); | ||
const { | ||
resetModules, | ||
itRenders, | ||
clientCleanRender, | ||
} = ReactDOMServerIntegrationUtils(initModules); | ||
|
||
describe('ReactDOMServerIntegration', () => { | ||
beforeEach(() => { | ||
|
@@ -488,21 +492,49 @@ describe('ReactDOMServerIntegration', () => { | |
); | ||
|
||
itRenders('badly cased SVG attribute with a warning', async render => { | ||
const e = await render(<text textlength="10" />, 1); | ||
expect(e.getAttribute('textLength')).toBe('10'); | ||
const e = await render( | ||
<svg> | ||
<text textlength="10" /> | ||
</svg>, | ||
1, | ||
); | ||
// The discrepancy is expected as long as we emit a warning | ||
// both on the client and the server. | ||
if (render === clientCleanRender) { | ||
// On the client, "textlength" is treated as a case-sensitive | ||
// SVG attribute so the wrong attribute ("textlength") gets set. | ||
expect(e.firstChild.getAttribute('textlength')).toBe('10'); | ||
expect(e.firstChild.hasAttribute('textLength')).toBe(false); | ||
} else { | ||
// When parsing HTML (including the hydration case), the browser | ||
// correctly maps "textlength" to "textLength" SVG attribute. | ||
// So it happens to work on the initial render. | ||
expect(e.firstChild.getAttribute('textLength')).toBe('10'); | ||
expect(e.firstChild.hasAttribute('textlength')).toBe(false); | ||
} | ||
}); | ||
|
||
itRenders('no badly cased aliased SVG attribute alias', async render => { | ||
const e = await render(<text strokedasharray="10 10" />, 1); | ||
expect(e.hasAttribute('stroke-dasharray')).toBe(false); | ||
expect(e.getAttribute('strokedasharray')).toBe('10 10'); | ||
const e = await render( | ||
<svg> | ||
<text strokedasharray="10 10" /> | ||
</svg>, | ||
1, | ||
); | ||
expect(e.firstChild.hasAttribute('stroke-dasharray')).toBe(false); | ||
expect(e.firstChild.getAttribute('strokedasharray')).toBe('10 10'); | ||
}); | ||
|
||
itRenders( | ||
'no badly cased original SVG attribute that is aliased', | ||
async render => { | ||
const e = await render(<text stroke-dasharray="10 10" />, 1); | ||
expect(e.getAttribute('stroke-dasharray')).toBe('10 10'); | ||
const e = await render( | ||
<svg> | ||
<text stroke-dasharray="10 10" /> | ||
</svg>, | ||
1, | ||
); | ||
expect(e.firstChild.getAttribute('stroke-dasharray')).toBe('10 10'); | ||
}, | ||
); | ||
}); | ||
|
@@ -558,6 +590,16 @@ describe('ReactDOMServerIntegration', () => { | |
); | ||
|
||
itRenders('custom attributes for non-standard elements', async render => { | ||
// This test suite generally assumes that we get exactly | ||
// the same warnings (or none) for all scenarios including | ||
// SSR + innerHTML, hydration, and client-side rendering. | ||
// However this particular warning fires only when creating | ||
// DOM nodes on the client side. We force it to fire early | ||
// so that it gets deduplicated later, and doesn't fail the test. | ||
expect(() => { | ||
ReactDOM.render(<nonstandard />, document.createElement('div')); | ||
}).toWarnDev('The tag <nonstandard> is unrecognized in this browser.'); | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Clever solution. 👍 |
||
|
||
const e = await render(<nonstandard foo="bar" />); | ||
expect(e.getAttribute('foo')).toBe('bar'); | ||
}); | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -17,7 +17,12 @@ function expectWarnings(tags, warnings = []) { | |
warnings = [...warnings]; | ||
|
||
let element = null; | ||
const container = document.createElement(tags.splice(0, 1)); | ||
const containerTag = tags.shift(); | ||
const container = | ||
containerTag === 'svg' | ||
? document.createElementNS('http://www.w3.org/2000/svg', containerTag) | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Nifty |
||
: document.createElement(containerTag); | ||
|
||
while (tags.length) { | ||
const Tag = tags.pop(); | ||
element = <Tag>{element}</Tag>; | ||
|
@@ -108,7 +113,6 @@ describe('validateDOMNesting', () => { | |
'validateDOMNesting(...): <body> cannot appear as a child of <foreignObject>.\n' + | ||
' in body (at **)\n' + | ||
' in foreignObject (at **)', | ||
'<foreignObject /> is using uppercase HTML', | ||
], | ||
); | ||
}); | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -32,6 +32,7 @@ import { | |
import assertValidProps from '../shared/assertValidProps'; | ||
import {DOCUMENT_NODE, DOCUMENT_FRAGMENT_NODE} from '../shared/HTMLNodeType'; | ||
import isCustomComponent from '../shared/isCustomComponent'; | ||
import possibleStandardNames from '../shared/possibleStandardNames'; | ||
import {validateProperties as validateARIAProperties} from '../shared/ReactDOMInvalidARIAHook'; | ||
import {validateProperties as validateInputProperties} from '../shared/ReactDOMNullInputValuePropHook'; | ||
import {validateProperties as validateUnknownProperties} from '../shared/ReactDOMUnknownPropertyHook'; | ||
|
@@ -813,6 +814,17 @@ export function updateProperties( | |
} | ||
} | ||
|
||
function getPossibleStandardName(propName: string): string | null { | ||
if (__DEV__) { | ||
const lowerCasedName = propName.toLowerCase(); | ||
if (!possibleStandardNames.hasOwnProperty(lowerCasedName)) { | ||
return null; | ||
} | ||
return possibleStandardNames[lowerCasedName] || null; | ||
} | ||
return null; | ||
} | ||
|
||
export function diffHydratedProperties( | ||
domElement: Element, | ||
tag: string, | ||
|
@@ -1017,6 +1029,7 @@ export function diffHydratedProperties( | |
isCustomComponentTag, | ||
) | ||
) { | ||
let isMismatchDueToBadCasing = false; | ||
if (propertyInfo !== null) { | ||
// $FlowFixMe - Should be inferred as not undefined. | ||
extraAttributeNames.delete(propertyInfo.attributeName); | ||
|
@@ -1035,6 +1048,17 @@ export function diffHydratedProperties( | |
// $FlowFixMe - Should be inferred as not undefined. | ||
extraAttributeNames.delete(propKey.toLowerCase()); | ||
} else { | ||
const standardName = getPossibleStandardName(propKey); | ||
if (standardName !== null && standardName !== propKey) { | ||
// If an SVG prop is supplied with bad casing, it will | ||
// be successfully parsed from HTML, but will produce a mismatch | ||
// (and would be incorrectly rendered on the client). | ||
// However, we already warn about bad casing elsewhere. | ||
// So we'll skip the misleading extra mismatch warning in this case. | ||
isMismatchDueToBadCasing = true; | ||
// $FlowFixMe - Should be inferred as not undefined. | ||
extraAttributeNames.delete(standardName); | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Nice! |
||
} | ||
// $FlowFixMe - Should be inferred as not undefined. | ||
extraAttributeNames.delete(propKey); | ||
} | ||
|
@@ -1045,7 +1069,7 @@ export function diffHydratedProperties( | |
); | ||
} | ||
|
||
if (nextProp !== serverValue) { | ||
if (nextProp !== serverValue && !isMismatchDueToBadCasing) { | ||
warnForPropDifference(propKey, serverValue, nextProp); | ||
} | ||
} | ||
|
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.
We weren't using these anymore
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.
expect
is still in here in beta, is that used?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.
Probably not..