Brijesh619 commented on code in PR #733:
URL: https://github.com/apache/atlas/pull/733#discussion_r3829542788
##########
dashboard/src/setupTests.simple.ts:
##########
@@ -19,6 +19,11 @@
import '@testing-library/jest-dom';
+// Polyfills for Node 12 / Jest jsdom compatibility
+import { TextEncoder, TextDecoder } from 'util';
Review Comment:
1. **Polyfill execution order & `export {}` placement:** I've completely
removed the polyfills from `setupTests.simple.ts`. They are now in a newly
created `src/setupTests.polyfills.ts` file, which is wired up via `setupFiles`
in `jest.config.js`. This ensures they run before test modules load and
resolves the `TextEncoder is not defined` crash. It also cleans up the ordering
confusion with `export {}` in the old setup file.
2. **Misleading Comment:** I've removed the outdated "Node 12" comment in
the new polyfills file and updated it to reflect that this is specifically for
Jest/jsdom compatibility.
3. **PR Description & Test Accuracy:** After applying the fix, I re-ran `npm
run test` locally. I can confirm the blocker is resolved, and all 189 suites /
4,790 tests are now passing successfully with a 100% success rate. I've updated
the PR description with the accurate counts.
4. **Node engines field:** Good catch on the version requirement. I have
added `"engines": { "node": ">=20" }` to `package.json` to explicitly document
and enforce the minimum Node version required for React Router v7, ensuring CI
and dev environments stay compatible.
##########
dashboard/src/setupTests.simple.ts:
##########
@@ -19,6 +19,11 @@
import '@testing-library/jest-dom';
+// Polyfills for Node 12 / Jest jsdom compatibility
Review Comment:
<img width="1366" height="768" alt="Screenshot from 2026-08-21 16-01-51"
src="https://github.com/user-attachments/assets/5ed85a3b-0dc2-4924-a778-dbf9dcf55a0b";
/>
I've fixed this by extracting the TextEncoder and TextDecoder polyfills into
a dedicated file (src/setupTests.polyfills.ts) and configuring it under the
setupFiles array in jest.config.js. This guarantees they execute before any
test modules load.
I've also removed the outdated "Node 12" comment as requested.
I just re-ran npm run test locally and can confirm that the crash is fully
resolved. All 189 suites and 4,790 tests are now passing successfully! I've
updated the PR description with the latest results.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
