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

Use modern rendering APIs for attribute-behavior fixture #27883

Merged
merged 6 commits into from
Jan 26, 2024
Merged

Use modern rendering APIs for attribute-behavior fixture #27883

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
6 commits
Select commit Hold shift + click to select a range
cc73a83
Ensure Apple Silicon support in attribute-behavior fixture
Jan 6, 2024
033fce1
Use modern rendering APIs for attribute-behavior fixture
Jan 6, 2024
c9b859a
Test `aria-hidden`
eps1lon Jul 17, 2022
1aba5a2
Test `inputMode` with defined value
eps1lon Jul 17, 2022
846dd5d
Test `inert`
eps1lon Jul 17, 2022
ea30662
Test hidden="until-found"
eps1lon Jul 17, 2022
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
2 changes: 1 addition & 1 deletion fixtures/attribute-behavior/.gitignore
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,7 @@
/build
/public/react.development.js
/public/react-dom.development.js
/public/react-dom-server-legacy.browser.development.js
/public/react-dom-server.browser.development.js

# misc
.DS_Store
Expand Down
132 changes: 91 additions & 41 deletions fixtures/attribute-behavior/AttributeTableSnapshot.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -294,7 +294,7 @@
| `action=(string 'on')`| (changed)| `"http://localhost:3000/on"` |
| `action=(string 'off')`| (changed)| `"http://localhost:3000/off"` |
| `action=(symbol)`| (initial, warning)| `"http://localhost:3000/"` |
| `action=(function)`| (initial, warning)| `"http://localhost:3000/"` |
| `action=(function)`| (changed, ssr error, ssr mismatch)| `"javascript:throw new Error('A React form was unexpectedly submitted. If you called form.submit() manually, consider using form.requestSubmit() instead. If you\'re trying to use event.stopPropagation() in a submit event handler, consider also calling event.preventDefault().')"` |
Copy link
Collaborator Author

@eps1lon eps1lon Jan 6, 2024 •
edited
Loading

Choose a reason for hiding this comment

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

Should I add a row for <input type="submit" action=* /> or rather add type="submit" to this row?

| `action=(null)`| (initial)| `"http://localhost:3000/"` |
| `action=(undefined)`| (initial)| `"http://localhost:3000/"` |

Expand Down Expand Up @@ -648,6 +648,31 @@
| `aria-=(null)`| (initial, warning)| `<null>` |
| `aria-=(undefined)`| (initial, warning)| `<null>` |

## `aria-hidden` (on `<div>` inside `<div>`)
| Test Case | Flags | Result |
| --- | --- | --- |
| `aria-hidden=(string)`| (changed)| `"a string"` |
| `aria-hidden=(empty string)`| (changed)| `<empty string>` |
| `aria-hidden=(array with string)`| (changed)| `"string"` |
| `aria-hidden=(empty array)`| (changed)| `<empty string>` |
| `aria-hidden=(object)`| (changed)| `"result of toString()"` |
| `aria-hidden=(numeric string)`| (changed)| `"42"` |
| `aria-hidden=(-1)`| (changed)| `"-1"` |
| `aria-hidden=(0)`| (changed)| `"0"` |
| `aria-hidden=(integer)`| (changed)| `"1"` |
| `aria-hidden=(NaN)`| (changed)| `"NaN"` |
| `aria-hidden=(float)`| (changed)| `"99.99"` |
| `aria-hidden=(true)`| (changed)| `"true"` |
| `aria-hidden=(false)`| (changed)| `"false"` |
| `aria-hidden=(string 'true')`| (changed)| `"true"` |
| `aria-hidden=(string 'false')`| (changed)| `"false"` |
| `aria-hidden=(string 'on')`| (changed)| `"on"` |
| `aria-hidden=(string 'off')`| (changed)| `"off"` |
| `aria-hidden=(symbol)`| (initial)| `<null>` |
| `aria-hidden=(function)`| (initial)| `<null>` |
| `aria-hidden=(null)`| (initial)| `<null>` |
| `aria-hidden=(undefined)`| (initial)| `<null>` |

## `aria-invalidattribute` (on `<div>` inside `<div>`)
| Test Case | Flags | Result |
| --- | --- | --- |
Expand Down Expand Up @@ -2568,8 +2593,8 @@
| `defaultChecked=(string 'false')`| (changed)| `<boolean: true>` |
| `defaultChecked=(string 'on')`| (changed)| `<boolean: true>` |
| `defaultChecked=(string 'off')`| (changed)| `<boolean: true>` |
| `defaultChecked=(symbol)`| (changed, ssr mismatch)| `<boolean: true>` |
| `defaultChecked=(function)`| (changed, ssr mismatch)| `<boolean: true>` |
| `defaultChecked=(symbol)`| (initial)| `<boolean: false>` |
| `defaultChecked=(function)`| (initial)| `<boolean: false>` |
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Is this ok?

| `defaultChecked=(null)`| (initial)| `<boolean: false>` |
| `defaultChecked=(undefined)`| (initial)| `<boolean: false>` |

Expand Down Expand Up @@ -4176,25 +4201,25 @@
## `formAction` (on `<input>` inside `<div>`)
| Test Case | Flags | Result |
| --- | --- | --- |
| `formAction=(string)`| (changed)| `"https://reactjs.com/"` |
| `formAction=(empty string)`| (initial)| `"http://localhost:3000/"` |
| `formAction=(array with string)`| (changed)| `"https://reactjs.com/"` |
| `formAction=(empty array)`| (initial)| `"http://localhost:3000/"` |
| `formAction=(object)`| (changed)| `"http://localhost:3000/result%20of%20toString()"` |
| `formAction=(numeric string)`| (changed)| `"http://localhost:3000/42"` |
| `formAction=(-1)`| (changed)| `"http://localhost:3000/-1"` |
| `formAction=(0)`| (changed)| `"http://localhost:3000/0"` |
| `formAction=(integer)`| (changed)| `"http://localhost:3000/1"` |
| `formAction=(string)`| (changed, warning)| `"https://reactjs.com/"` |
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

logs Warning: An input can only specify a formAction along with type="submit" or type="image".%s. Should I add a row for <input type="submit" formAction=* /> or rather add type="submit" to this row?

| `formAction=(empty string)`| (initial, warning)| `"http://localhost:3000/"` |
| `formAction=(array with string)`| (changed, warning)| `"https://reactjs.com/"` |
| `formAction=(empty array)`| (initial, warning)| `"http://localhost:3000/"` |
| `formAction=(object)`| (changed, warning)| `"http://localhost:3000/result%20of%20toString()"` |
| `formAction=(numeric string)`| (changed, warning)| `"http://localhost:3000/42"` |
| `formAction=(-1)`| (changed, warning)| `"http://localhost:3000/-1"` |
| `formAction=(0)`| (changed, warning)| `"http://localhost:3000/0"` |
| `formAction=(integer)`| (changed, warning)| `"http://localhost:3000/1"` |
| `formAction=(NaN)`| (changed, warning)| `"http://localhost:3000/NaN"` |
| `formAction=(float)`| (changed)| `"http://localhost:3000/99.99"` |
| `formAction=(float)`| (changed, warning)| `"http://localhost:3000/99.99"` |
| `formAction=(true)`| (initial, warning)| `"http://localhost:3000/"` |
| `formAction=(false)`| (initial, warning)| `"http://localhost:3000/"` |
| `formAction=(string 'true')`| (changed)| `"http://localhost:3000/true"` |
| `formAction=(string 'false')`| (changed)| `"http://localhost:3000/false"` |
| `formAction=(string 'on')`| (changed)| `"http://localhost:3000/on"` |
| `formAction=(string 'off')`| (changed)| `"http://localhost:3000/off"` |
| `formAction=(string 'true')`| (changed, warning)| `"http://localhost:3000/true"` |
| `formAction=(string 'false')`| (changed, warning)| `"http://localhost:3000/false"` |
| `formAction=(string 'on')`| (changed, warning)| `"http://localhost:3000/on"` |
| `formAction=(string 'off')`| (changed, warning)| `"http://localhost:3000/off"` |
| `formAction=(symbol)`| (initial, warning)| `"http://localhost:3000/"` |
| `formAction=(function)`| (initial, warning)| `"http://localhost:3000/"` |
| `formAction=(function)`| (changed, warning, ssr error, ssr mismatch)| `"javascript:throw new Error('A React form was unexpectedly submitted. If you called form.submit() manually, consider using form.requestSubmit() instead. If you\'re trying to use event.stopPropagation() in a submit event handler, consider also calling event.preventDefault().')"` |
| `formAction=(null)`| (initial)| `"http://localhost:3000/"` |
| `formAction=(undefined)`| (initial)| `"http://localhost:3000/"` |

Expand Down Expand Up @@ -5052,7 +5077,7 @@
| Test Case | Flags | Result |
| --- | --- | --- |
| `href=(string)`| (changed)| `"https://reactjs.com/"` |
| `href=(empty string)`| (changed)| `"http://localhost:3000/"` |
| `href=(empty string)`| (initial, warning)| `<empty string>` |
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Logs An empty string ("") was passed to the href attribute. To fix this, either do not render the element at all or pass null to href instead of an empty string..

This seems like a bug since <a href={null} /> would also not set the href to the base url. <a href="" /> is a pretty common "Home" link I believe.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

I believe this was introduced in #26379. @sebmarkbage could you clarify if this is intended?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

It's a bug. Will work on a fix.

| `href=(array with string)`| (changed)| `"https://reactjs.com/"` |
| `href=(empty array)`| (changed)| `"http://localhost:3000/"` |
| `href=(object)`| (changed)| `"http://localhost:3000/result%20of%20toString()"` |
Expand Down Expand Up @@ -5398,6 +5423,31 @@
| `in=(null)`| (initial)| `<null>` |
| `in=(undefined)`| (initial)| `<null>` |

## `inert` (on `<div>` inside `<div>`)
| Test Case | Flags | Result |
| --- | --- | --- |
| `inert=(string)`| (changed)| `<boolean: true>` |
| `inert=(empty string)`| (changed)| `<boolean: true>` |
| `inert=(array with string)`| (changed)| `<boolean: true>` |
| `inert=(empty array)`| (changed)| `<boolean: true>` |
| `inert=(object)`| (changed)| `<boolean: true>` |
| `inert=(numeric string)`| (changed)| `<boolean: true>` |
| `inert=(-1)`| (changed)| `<boolean: true>` |
| `inert=(0)`| (changed)| `<boolean: true>` |
| `inert=(integer)`| (changed)| `<boolean: true>` |
| `inert=(NaN)`| (changed, warning)| `<boolean: true>` |
| `inert=(float)`| (changed)| `<boolean: true>` |
| `inert=(true)`| (initial, warning)| `<boolean: false>` |
| `inert=(false)`| (initial, warning)| `<boolean: false>` |
| `inert=(string 'true')`| (changed)| `<boolean: true>` |
| `inert=(string 'false')`| (changed)| `<boolean: true>` |
| `inert=(string 'on')`| (changed)| `<boolean: true>` |
| `inert=(string 'off')`| (changed)| `<boolean: true>` |
| `inert=(symbol)`| (initial, warning)| `<boolean: false>` |
| `inert=(function)`| (initial, warning)| `<boolean: false>` |
| `inert=(null)`| (initial)| `<boolean: false>` |
| `inert=(undefined)`| (initial)| `<boolean: false>` |

## `in2` (on `<feBlend>` inside `<svg>`)
| Test Case | Flags | Result |
| --- | --- | --- |
Expand Down Expand Up @@ -5501,27 +5551,27 @@
## `inputMode` (on `<input>` inside `<div>`)
| Test Case | Flags | Result |
| --- | --- | --- |
| `inputMode=(string)`| (changed)| `"a string"` |
| `inputMode=(empty string)`| (changed)| `<empty string>` |
| `inputMode=(array with string)`| (changed)| `"string"` |
| `inputMode=(empty array)`| (changed)| `<empty string>` |
| `inputMode=(object)`| (changed)| `"result of toString()"` |
| `inputMode=(numeric string)`| (changed)| `"42"` |
| `inputMode=(-1)`| (changed)| `"-1"` |
| `inputMode=(0)`| (changed)| `"0"` |
| `inputMode=(integer)`| (changed)| `"1"` |
| `inputMode=(NaN)`| (changed, warning)| `"NaN"` |
| `inputMode=(float)`| (changed)| `"99.99"` |
| `inputMode=(true)`| (initial, warning)| `<null>` |
| `inputMode=(false)`| (initial, warning)| `<null>` |
| `inputMode=(string 'true')`| (changed)| `"true"` |
| `inputMode=(string 'false')`| (changed)| `"false"` |
| `inputMode=(string 'on')`| (changed)| `"on"` |
| `inputMode=(string 'off')`| (changed)| `"off"` |
| `inputMode=(symbol)`| (initial, warning)| `<null>` |
| `inputMode=(function)`| (initial, warning)| `<null>` |
| `inputMode=(null)`| (initial)| `<null>` |
| `inputMode=(undefined)`| (initial)| `<null>` |
| `inputMode=(string)`| (initial)| `<empty string>` |
| `inputMode=(empty string)`| (initial)| `<empty string>` |
| `inputMode=(array with string)`| (initial)| `<empty string>` |
| `inputMode=(empty array)`| (initial)| `<empty string>` |
| `inputMode=(object)`| (initial)| `<empty string>` |
| `inputMode=(numeric string)`| (initial)| `<empty string>` |
| `inputMode=(-1)`| (initial)| `<empty string>` |
| `inputMode=(0)`| (initial)| `<empty string>` |
| `inputMode=(integer)`| (initial)| `<empty string>` |
| `inputMode=(NaN)`| (initial, warning)| `<empty string>` |
| `inputMode=(float)`| (initial)| `<empty string>` |
| `inputMode=(true)`| (initial, warning)| `<empty string>` |
| `inputMode=(false)`| (initial, warning)| `<empty string>` |
| `inputMode=(string 'true')`| (initial)| `<empty string>` |
| `inputMode=(string 'false')`| (initial)| `<empty string>` |
| `inputMode=(string 'on')`| (initial)| `<empty string>` |
| `inputMode=(string 'off')`| (initial)| `<empty string>` |
| `inputMode=(symbol)`| (initial, warning)| `<empty string>` |
| `inputMode=(function)`| (initial, warning)| `<empty string>` |
| `inputMode=(null)`| (initial)| `<empty string>` |
| `inputMode=(undefined)`| (initial)| `<empty string>` |

## `integrity` (on `<script>` inside `<div>`)
| Test Case | Flags | Result |
Expand Down Expand Up @@ -9877,7 +9927,7 @@
| Test Case | Flags | Result |
| --- | --- | --- |
| `src=(string)`| (changed)| `"https://reactjs.com/"` |
| `src=(empty string)`| (changed)| `"http://localhost:3000/"` |
| `src=(empty string)`| (initial, warning)| `<empty string>` |
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Logs An empty string ("") was passed to the src attribute. This may cause the browser to download the whole page again over the network. To fix this, either do not render the element at all or pass null to src instead of an empty string..

I think this was added intentionally?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

It was. Will land enableFilterEmptyStringAttributesDOM on main next.

| `src=(array with string)`| (changed)| `"https://reactjs.com/"` |
| `src=(empty array)`| (changed)| `"http://localhost:3000/"` |
| `src=(object)`| (changed)| `"http://localhost:3000/result%20of%20toString()"` |
Expand Down
5 changes: 4 additions & 1 deletion fixtures/attribute-behavior/package.json
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -10,9 +10,12 @@
"react-scripts": "1.0.11",
"react-virtualized": "^9.9.0"
},
"resolutions": {
"fsevents": "1.2.13"
},
"scripts": {
"predev":
"cp ../../build/oss-experimental/react/umd/react.development.js public/ && cp ../../build/oss-experimental/react-dom/umd/react-dom.development.js public/ && cp ../../build/oss-experimental/react-dom/umd/react-dom-server-legacy.browser.development.js public/",
"cp ../../build/oss-experimental/react/umd/react.development.js public/ && cp ../../build/oss-experimental/react-dom/umd/react-dom.development.js public/ && cp ../../build/oss-experimental/react-dom/umd/react-dom-server.browser.development.js public/",
"dev": "react-scripts start",
"build": "react-scripts build",
"test": "react-scripts test --env=jsdom",
Expand Down
68 changes: 50 additions & 18 deletions fixtures/attribute-behavior/src/App.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -213,13 +213,32 @@ function getCanonicalizedValue(value) {

let _didWarn = false;
function warn(str) {
if (str.includes('ReactDOM.render is no longer supported')) {
return;
}
_didWarn = true;
}

/**
* @param {import('react-dom/server')} serverRenderer
*/
async function renderToString(serverRenderer, element) {
let didError = false;
const stream = await serverRenderer.renderToReadableStream(element, {
onError(error) {
didError = true;
console.error(error);
},
});
await stream.allReady;

if (didError) {
throw new Error('The above error occurred while rendering to string.');
}

const response = new Response(stream);
return response.text();
}

const UNKNOWN_HTML_TAGS = new Set(['keygen', 'time', 'command']);
function getRenderedAttributeValue(
async function getRenderedAttributeValue(
react,
renderer,
serverRenderer,
Expand Down Expand Up @@ -283,12 +302,21 @@ function getRenderedAttributeValue(
_didWarn = false;
try {
let container = createContainer();
renderer.render(react.createElement(tagName, baseProps), container);
renderer.flushSync(() => {
renderer
.createRoot(container)
.render(react.createElement(tagName, baseProps));
});
defaultValue = read(container.lastChild);
canonicalDefaultValue = getCanonicalizedValue(defaultValue);

container = createContainer();
renderer.render(react.createElement(tagName, props), container);

renderer.flushSync(() => {
renderer
.createRoot(container)
.render(react.createElement(tagName, props));
});
result = read(container.lastChild);
canonicalResult = getCanonicalizedValue(result);
didWarn = _didWarn;
Expand All @@ -305,19 +333,22 @@ function getRenderedAttributeValue(
try {
let container;
if (containerTagName === 'document') {
const html = serverRenderer.renderToString(
const html = await renderToString(
serverRenderer,
react.createElement(tagName, props)
);
container = createContainer();
container.innerHTML = html;
} else if (containerTagName === 'head') {
const html = serverRenderer.renderToString(
const html = await renderToString(
serverRenderer,
react.createElement(tagName, props)
);
container = createContainer();
container.innerHTML = html;
} else {
const html = serverRenderer.renderToString(
const html = await renderToString(
serverRenderer,
react.createElement(
containerTagName,
null,
Expand All @@ -326,7 +357,8 @@ function getRenderedAttributeValue(
);
const outerContainer = document.createElement('div');
outerContainer.innerHTML = html;
container = outerContainer.firstChild;
// Float may prepend `<link />`
container = outerContainer.lastChild;
}

if (
Expand Down Expand Up @@ -396,8 +428,8 @@ function getRenderedAttributeValue(
};
}

function prepareState(initGlobals) {
function getRenderedAttributeValues(attribute, type) {
async function prepareState(initGlobals) {
async function getRenderedAttributeValues(attribute, type) {
const {
ReactStable,
ReactDOMStable,
Expand All @@ -406,14 +438,14 @@ function prepareState(initGlobals) {
ReactDOMNext,
ReactDOMServerNext,
} = initGlobals(attribute, type);
const reactStableValue = getRenderedAttributeValue(
const reactStableValue = await getRenderedAttributeValue(
ReactStable,
ReactDOMStable,
ReactDOMServerStable,
attribute,
type
);
const reactNextValue = getRenderedAttributeValue(
const reactNextValue = await getRenderedAttributeValue(
ReactNext,
ReactDOMNext,
ReactDOMServerNext,
Expand Down Expand Up @@ -451,7 +483,7 @@ function prepareState(initGlobals) {
let hasSameBehaviorForAll = true;
let rowPatternHash = '';
for (let type of types) {
const result = getRenderedAttributeValues(attribute, type);
const result = await getRenderedAttributeValues(attribute, type);
results.set(type.name, result);
if (!result.hasSameBehavior) {
hasSameBehaviorForAll = false;
Expand Down Expand Up @@ -772,10 +804,10 @@ class App extends React.Component {
ReactDOMStable:
'https://unpkg.com/react-dom@latest/umd/react-dom.development.js',
ReactDOMServerStable:
'https://unpkg.com/react-dom@latest/umd/react-dom-server-legacy.browser.development.js',
'https://unpkg.com/react-dom@latest/umd/react-dom-server.browser.development.js',
ReactNext: '/react.development.js',
ReactDOMNext: '/react-dom.development.js',
ReactDOMServerNext: '/react-dom-server-legacy.browser.development.js',
ReactDOMServerNext: '/react-dom-server.browser.development.js',
};
const codePromises = Object.values(sources).map(src =>
fetch(src).then(res => res.text())
Expand Down Expand Up @@ -820,7 +852,7 @@ class App extends React.Component {
return globals;
}

const {table, rowPatternHashes} = prepareState(initGlobals);
const {table, rowPatternHashes} = await prepareState(initGlobals);
document.title = 'Ready';

this.setState({
Expand Down
Loading