Update attribute-behavior fixture#22522
Conversation
|
Comparing: 4e6eec6...293f9e8 Critical size changesIncludes critical production bundles, as well as any change greater than 2%:
Significant size changesIncludes any change greater than 0.2%: (No significant changes) |
…vior/update-fixture
| | `value=(string 'on')`| (changed)| `"on"` | | ||
| | `value=(string 'off')`| (changed)| `"off"` | | ||
| | `value=(symbol)`| (initial, warning)| `<empty string>` | | ||
| | `value=(symbol)`| (initial, warning, ssr error, ssr mismatch)| `<empty string>` | |
There was a problem hiding this comment.
Warning on client and server in React 17: https://codesandbox.io/s/react-17-option-value-symbol-cy2rf
Throws on server in React 18 but also has an additional warning: https://codesandbox.io/s/react-18-option-value-symbol-kbc2n
There was a problem hiding this comment.
Interesting. Adding to TODOs to ensure this is intentional.
| | `selected=(string 'on')`| (initial, warning, ssr warning)| `<boolean: true>` | | ||
| | `selected=(string 'off')`| (initial, warning, ssr warning)| `<boolean: true>` | | ||
| | `selected=(string 'on')`| (initial, warning)| `<boolean: true>` | | ||
| | `selected=(string 'off')`| (initial, warning)| `<boolean: true>` | |
There was a problem hiding this comment.
React 17 only had a warning on the client: https://codesandbox.io/s/react-17-option-selected-on-5nl1q
React 18 has a warning on client and server: https://codesandbox.io/s/react-18-option-selected-on-9ciud
I think the "ssr warning" is a bit misleading here in my opinion. If the snapshot reads "ssr warning" I would expect a warning on the server. But the opposite is the case here.
Assuming that the other "ssr warning" changes follow the same pattern so not investigating those further
Just to clarify, does this mean the browser behavior changed in the meantime for these other changes? |
Based on the assumption that the last time the file was changed, I committed all changes and not just the ones for |
| | `value=(array with string)`| (changed)| `"string"` | | ||
| | `value=(empty array)`| (initial)| `<empty string>` | | ||
| | `value=(object)`| (changed)| `"result of toString()"` | | ||
| | `value=(object)`| (changed, ssr error, ssr mismatch)| `"result of toString()"` | |
There was a problem hiding this comment.
This is a new throw. I'll add this to a list of things to check before the release.
| | `defaultValue=(string 'off')`| (changed)| `"off"` | | ||
| | `defaultValue=(symbol)`| (initial, ssr warning)| `<empty string>` | | ||
| | `defaultValue=(function)`| (initial, ssr warning)| `<empty string>` | | ||
| | `defaultValue=(null)`| (initial, ssr warning)| `<empty string>` | |
There was a problem hiding this comment.
Looked at this one. It's because old SSR always assigned this prop internally and so it went through the value validation which complains about null. Seems fine that it doesn't do that now.
| ## `amplitude` (on `<feFuncA>` inside `<svg>`) | ||
| | Test Case | Flags | Result | | ||
| | --- | --- | --- | | ||
| | `amplitude=(string)`| (changed)| `<number: 0>` | |
There was a problem hiding this comment.
Looked at this one. Appears to be a browser behavior change. Chrome warns in console about invalid value.
| | `style=(string)`| (changed, error, warning, ssr error)| `` | | ||
| | `style=(empty string)`| (changed, error, warning, ssr error)| `` | | ||
| | `style=(array with string)`| (initial)| `[]` | | ||
| | `style=(array with string)`| (changed, error, warning, ssr mismatch)| `` | |
There was a problem hiding this comment.
This seems like a browser behavior change.
gaearon
left a comment
gaearon
left a comment
There was a problem hiding this comment.
Looks good. I've added the suspicious ones to pre-release checklist to follow up on.
* Fix missing key warning * Add build instructions * Update interpretation now that React 17 is latest stable and 18 is next * Ignore ReactDOM.render deprecation warning * Ensure a server implementation with `renderToString` is used * Update AttributeTableSnapshot * Ensure Popover doesn't overflow
6 participants
Summary
Notable changes:
Wanted to add
imageSizesandimageSrcSetand noticed that theattribute-behaviorfixture is outdated.Fixture will still use the same APIs for 17 and next. Instead of changing APIs we just work around legacy warnings/errors.
Individual commits describe changes in more detail.
How did you test this change?
Followed (updated) setup instructions of
fixtures/attribute-behavior/README.mdusingnode@12.22.6and Chrome Version 96.0.4664.45Why
AttributeTableSnapshotchanged?When creating a snapshot using the last commit where we updated the snapshot (feb134c), we get the following diff:
Changes on feb134c
If we subtract that diff from the diff of this PR we're left with the following changes:
I annotated those diffs with minimal repros.