FED-3528 React 18 dual support #84
Conversation
+ Added interop for new filterNode option for prettyDom + Fixed misc. tests — JS CHANGELOGS — - https://github.com/testing-library/dom-testing-library/releases/tag/v8.0.0 - https://github.com/testing-library/react-testing-library/releases/tag/v12.0.0
Derived from building master (3.0.2) with the following changes:
diff --git a/js_src/rollup.config.js b/js_src/rollup.config.js
index 38448f5..e8432dfc 100644
--- a/js_src/rollup.config.js
+++ b/js_src/rollup.config.js
@@ -68 +68 @@ export default (commandFlags) => {
- format: 'umd',
+ format: 'esm',
This reverts commit fd02c66.
# Conflicts: # pubspec.yaml
# Conflicts: # .github/workflows/dart_ci.yml
sydneyjodon-wk
requested review from
greglittlefield-wf,
kealjones-wk and
aaronlademann-wf
js_src/vite.config.mjs
Outdated
|
|
||
| export default defineConfig({ | ||
| define: { | ||
| 'process.env.NODE_ENV': JSON.stringify('production'), |
There was a problem hiding this comment.
Hmm I never noticed this, but I would think for this package that we'd actually want this to be 'development'... Since we'd want more assertions in a test environment, right?
Wonder what rollup was doing before. Or, if it really makes much of a difference; looks like there's not a lot of checking of these at least in RTL itself: https://github.com/search?q=repo%3Atesting-library%2Freact-testing-library%20NODE_ENV&type=code
So we're probably good to just switch it?
| @@ -109,6 +109,7 @@ RenderResult render( | |||
| Node? container, | |||
| Node? baseElement, | |||
| bool hydrate = false, | |||
| bool legacyRoot = true, | |||
There was a problem hiding this comment.
Could we add legacyRoot to the doc comment?
Could go based off the site's docs: https://testing-library.com/docs/react-testing-library/api/#legacyroot
(Though reminder our default value is different from theirs)
js_src/package.json
Outdated
| "@testing-library/dom": "^8.0.0", | ||
| "@testing-library/react": "^13.0.0", |
There was a problem hiding this comment.
#nit could we up these bounds to match what we resolved to, to help ensure we don't go backwards?
| "@testing-library/dom": "^8.0.0", | |
| "@testing-library/react": "^13.0.0", | |
| "@testing-library/dom": "^8.20.1", | |
| "@testing-library/react": "^13.4.0", |
| /// You can configure this behavior by passing a custom `filterNode` function that should return true for every | ||
| /// node that you wish to include in the output. | ||
| /// {@endtemplate} | ||
| external bool Function(Node)? get filterNode; |
There was a problem hiding this comment.
Could we add a basic test for this option in pretty_dom_test.dart?
Motivation
We'll need to set up dual support for React 17 and 18 in react_testing_library (RTL).
Changes
Release Notes
Review
See CONTRIBUTING.md for more details on review types (+1 / QA +1 / +10) and code review process.
QA Checklist
Merge Checklist
While we perform many automated checks before auto-merging, some manual checks are needed: