bfeister
commented
2 months ago Copying the discussion with @shethj for future reference for anyone who looks over this PR:
====
Brian Feister 11:39 AM Ok, so for the jest-environment-jsdom that gets used in our global jest.config.js in internal-lib-build/configs/jest/jest.config.js Soooooo, this means it technically wraps the configuration for ALL of our testing right? What happens when you don't use the polyfill?
Jainam Sheth :spiral_calendar_pad: 11:40 AM What happens when you don’t use the polyfill? This causes our test here to fail with error: ReferenceError: setImmediate is not defined express.test.js test('serveStaticFile returns 404 if the file does not exist', () => { const app = RemoteServerFactory._createApp(opts())
app.get('/thing', RemoteServerFactory.serveStaticFile('this-does-not-exist.ico'))setImmedeate is a node thing not a jest thing. It is used internally for scheduling test execution.
So you either use a polyfill for setImmedeate or you switch your jestEnvironment to node or you stay at v26.x
https://salesforce-internal.slack.com/archives/C07FNSQMKD3/p1722966072917589 switching environments will break more stuff
Brian Feister
Yeah I get that, but I think we want to know "how much stuff" we're breaking
I say that because our express / Node testing layer is thin so the impact could be small and we might not be stuck on old deps
Jainam Sheth
Use Node: Testing Node.js modules, servers, or utilities. Testing code that doesn’t rely on browser APIs or the DOM. Achieving faster test execution speeds (typically). Use jsdom: Testing frontend code that manipulates the DOM. Testing code that relies on browser-specific APIs like window, document, fetch, etc. Simulating user interactions with the DOM. I think we need the env to be jsdom
Brian Feister
Interesting, so do all current / new-ish React projects use the polyfill then?
I've been reading lately about how Google and other companies are trying to get away from jsdom
It might be worth looking around to see if say Next.js for example, uses the polyfill for jsdom in their testing
Jainam Sheth
I saw this bug on prisma: https://github.com/prisma/prisma/issues/8558 There are mixed opinions. One interesting comment is: “The way I see it, the fact that Jest 26 supported setImmediate in jsdom was a mistake corrected in Jest 27.” (link)
8558 Jest 27 errors with ReferenceError: setImmediate is not defined
https://github.com/prisma/prisma|prisma/prismaprisma/prisma | Aug 3rd, 2021 | Added by https://salesforce-internal.slack.com/services/B01UMHCT5AQ
Comment on #8558 Jest 27 errors with ReferenceError: setImmediate is not defined
https://github.com/[prisma/prisma](https://github.com/prisma/prisma)|prisma/prismaprisma/prisma | Jan 20th, 2022 | Added by https://salesforce-internal.slack.com/services/B01UMHCT5AQ
Brian Feister
Yeah, wow lots of opinions
What is your gut feeling about it after studying the implications?
Is it as simple as doing this with the polyfill?
//@ts-ignore
window.setImmediate = window.setTimeoutThe file we’re seeing failing test is express.test.js which tests server APIs and satisfies use cases for testEnv to be node outlined here:
Use Node:
Testing Node.js modules, servers, or utilities.
Testing code that doesn't rely on browser APIs or the DOM.
Achieving faster test execution speeds (typically).
Use jsdom:
Testing frontend code that manipulates the DOM.
Testing code that relies on browser-specific APIs like window, document, fetch, etc.
Simulating user interactions with the DOM.How do you feel about switching testEnvironments to node just for that test file ?
However I read setImmedeate is only supported in IE10 and exists in Node for now This doesn't matter because this is only for our testing environment, it won't impact customers, right?
Jainam Sheth 12:01 PM Nope. Won’t impact customers (edited)
Brian Feister Yeah so not a factor then
Jainam Sheth So then polyfill or set per-file env or not upgrade at all lol?
Brian Feister
How do you feel about switching testEnvironments to node just for that test file ? It seems that since we're testing express.js node would be the more correct testing env, so yeah I like that idea
Jainam Sheth
Cool. I can check to see if that works.
Brian Feister
Sounds good. Sounds like a big research project. Well done. Let's ship it if we can get a working solution
This PR address some dependency upgrades in the monorepo as suggested by Snyk
Description
Non-Breaking Changes
Breaking Changes
The following dependencies have been skipped:
What breaks ?
jest, jest-cli, jest-environment-jsdom, babel-jest seem to upgrade fine.
bumping jest to 29.7.0 requires jest-environment-jsdom-global to be bumped to 4.0.0 as it uses jest as a peer dependency
There are lots of changes regarding setImmediate in the jest changelog for jest 27 https://github.com/facebook/jest/blob/master/CHANGELOG.md
This causes our test here to fail with error:
ReferenceError: setImmediate is not definedPossibel fixes:
setImmedeateORtestEnvironmentin jest tonodeinstead ofjsdomTypes of Changes
Changes
How to Test-Drive This PR
Checklists
General
Accessibility Compliance
You must check off all items in one of the follow two lists:
or...
Localization