Bump @tootallnate/once, jest-environment-jsdom and jest-preset-angular in /gui-js#618
Bump @tootallnate/once, jest-environment-jsdom and jest-preset-angular in /gui-js#618dependabot[bot] wants to merge 4 commits intomasterfrom
Conversation
Removes [@tootallnate/once](https://github.com/TooTallNate/once). It's no longer used after updating ancestor dependencies [@tootallnate/once](https://github.com/TooTallNate/once), [jest-environment-jsdom](https://github.com/jestjs/jest/tree/HEAD/packages/jest-environment-jsdom) and [jest-preset-angular](https://github.com/thymikee/jest-preset-angular). These dependencies need to be updated together. Removes `@tootallnate/once` Updates `jest-environment-jsdom` from 29.7.0 to 30.2.0 - [Release notes](https://github.com/jestjs/jest/releases) - [Changelog](https://github.com/jestjs/jest/blob/main/CHANGELOG.md) - [Commits](https://github.com/jestjs/jest/commits/v30.2.0/packages/jest-environment-jsdom) Updates `jest-preset-angular` from 14.6.2 to 16.1.1 - [Release notes](https://github.com/thymikee/jest-preset-angular/releases) - [Changelog](https://github.com/thymikee/jest-preset-angular/blob/main/CHANGELOG.md) - [Commits](thymikee/jest-preset-angular@v14.6.2...v16.1.1) --- updated-dependencies: - dependency-name: "@tootallnate/once" dependency-version: dependency-type: indirect - dependency-name: jest-environment-jsdom dependency-version: 30.2.0 dependency-type: direct:development - dependency-name: jest-preset-angular dependency-version: 16.1.1 dependency-type: direct:development ... Signed-off-by: dependabot[bot] <support@github.com>
|
@CodeRabbit review |
✅ Actions performedReview triggered.
|
|
Important Review skippedBot user detected. To trigger a single review, invoke the ⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Run ID: You can disable this status message by setting the Use the checkbox below for a quick retry:
📝 WalkthroughWalkthroughThis PR updates dependencies (Angular 20→21, TypeScript 5.8.3→5.9.0, Jest 29→30), refactors loading spinner logic to stop after first navigation, updates test environment setup to use zone-based Jest preset, removes redundant HttpClientModule, and adds TypeScript bundler module resolution configuration. Changes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Poem
🚥 Pre-merge checks | ✅ 3✅ Passed checks (3 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
There was a problem hiding this comment.
Actionable comments posted: 1
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (3)
gui-js/package.json (2)
169-176:⚠️ Potential issue | 🟠 MajorFinish the Jest 30 migration across the remaining projects.
Line 169 still pins
@types/jestto 29 while Lines 174-176 move the runtime stack to 30. The repo also still has legacysetup-jest/globals['ts-jest']config ingui-js/apps/minsky-electron/src/test-setup.tsandgui-js/libs/menu/jest.config.ts. Jest 30 already ships TypeScript declarations,@types/jest30 exists, and the currentjest-preset-angular16.x docs usesetupZoneTestEnv()plus preset/transform helpers, so this upgrade is still only partially migrated. (npmjs.com)Run this to confirm the remaining legacy Jest config:
#!/bin/bash set -euo pipefail echo "Legacy setup-jest imports:" rg -n "jest-preset-angular/setup-jest" gui-js echo echo "Jest configs still using globals['ts-jest']:" rg -n -C2 "globals\\s*:\\s*\\{|['\"]ts-jest['\"]" gui-js -g 'jest.config.*'🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@gui-js/package.json` around lines 169 - 176, Update the pinned `@types/jest` in package.json to the v30 range (e.g. ^30.0.0) and remove legacy setup-jest/ts-jest globals across the GUI project; specifically, delete or stop importing "jest-preset-angular/setup-jest" in gui-js/apps/minsky-electron/src/test-setup.ts and remove any globals['ts-jest'] or ts-jest entries in gui-js/libs/menu/jest.config.ts (and other jest.config.* files), then adopt the Jest-30 / jest-preset-angular-16 migration pattern by using setupZoneTestEnv() in your test setup and switching to the preset/transform helpers recommended by the preset docs so runtime and types are consistently using Jest 30.
158-205:⚠️ Potential issue | 🟠 MajorRaise the Node engine floor to match Angular 21's compatibility requirements.
The current
engines.nodespecification of>=20.11.1permits Node versions that Angular 21 no longer supports. Angular 21 requires Node^20.19.0 || ^22.12.0 || ^24.0.0, making any Node version from 20.11.1 through 20.18.x incompatible with the framework.Suggested fix
"engines": { - "node": ">=20.11.1" + "node": "^20.19.0 || ^22.12.0 || ^24.0.0" },🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@gui-js/package.json` around lines 158 - 205, The engines.node field in package.json currently allows ">=20.11.1", which permits Node versions incompatible with Angular 21; update the engines.node value to match Angular 21's supported range (e.g. "^20.19.0 || ^22.12.0 || ^24.0.0") so that the package.json enforces only compatible Node versions; locate and change the "engines": { "node": ... } entry in package.json accordingly and run your usual install/CI to verify no version conflicts.gui-js/apps/minsky-web/src/test-setup.ts (1)
1-19:⚠️ Potential issue | 🟠 MajorUse the supported
jest-preset-angular16 setup API.The project uses
jest-preset-angular^16.1.1, which documentssetupZoneTestEnv()as the standard way to initialize the Angular test environment. The current code mixes three distinct patterns (manualglobalThis.ngJestassignment, side-effect import, and explicitTestBed.initTestEnvironment()call), which is inconsistent with the 16.x recommended approach. Consolidate onsetupZoneTestEnv(), which accepts the test environment options directly.Suggested fix
-globalThis.ngJest = { - testEnvironmentOptions: { - errorOnUnknownElements: true, - errorOnUnknownProperties: true, - }, - }; -import 'jest-preset-angular/setup-env/zone'; +import { setupZoneTestEnv } from 'jest-preset-angular/setup-env/zone'; import '../../../libs/shared/src/test-setup'; - -import { TestBed } from '@angular/core/testing'; -import { BrowserDynamicTestingModule, platformBrowserDynamicTesting } from '@angular/platform-browser-dynamic/testing'; - -// Initialize the Angular testing environment for Jest. -// Without this, TestBed.configureTestingModule() throws: -// "Need to call TestBed.initTestEnvironment() first" -TestBed.initTestEnvironment( - BrowserDynamicTestingModule, - platformBrowserDynamicTesting(), -); +setupZoneTestEnv({ + errorOnUnknownElements: true, + errorOnUnknownProperties: true, +});🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@gui-js/apps/minsky-web/src/test-setup.ts` around lines 1 - 19, Replace the mixed/legacy test init pattern with the jest-preset-angular v16 API: remove the manual globalThis.ngJest assignment, drop the direct side-effect import 'jest-preset-angular/setup-env/zone', and remove the explicit TestBed.initTestEnvironment(...) call; instead call setupZoneTestEnv(...) from 'jest-preset-angular' passing the testEnvironmentOptions (errorOnUnknownElements, errorOnUnknownProperties) and BrowserDynamicTestingModule/platformBrowserDynamicTesting as needed so the Angular TestBed is initialized through setupZoneTestEnv; update imports to reference setupZoneTestEnv and remove unused symbols (globalThis.ngJest, TestBed.initTestEnvironment, setup-env/zone).
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@gui-js/apps/minsky-web/src/app/app.component.ts`:
- Around line 58-62: The ngDoCheck method currently clears this.loading too
early, so remove the line "this.loading = false;" from ngDoCheck and let the
NavigationEnd subscription (where you already set this.loading to false) be the
single owner of the spinner state; keep updatePubTabs() and
this.cdRef.detectChanges() in ngDoCheck but ensure only the NavigationEnd
handler sets this.loading to false so the loader closes after successful
navigation (refer to ngDoCheck, updatePubTabs, cdRef.detectChanges and the
NavigationEnd subscription).
---
Outside diff comments:
In `@gui-js/apps/minsky-web/src/test-setup.ts`:
- Around line 1-19: Replace the mixed/legacy test init pattern with the
jest-preset-angular v16 API: remove the manual globalThis.ngJest assignment,
drop the direct side-effect import 'jest-preset-angular/setup-env/zone', and
remove the explicit TestBed.initTestEnvironment(...) call; instead call
setupZoneTestEnv(...) from 'jest-preset-angular' passing the
testEnvironmentOptions (errorOnUnknownElements, errorOnUnknownProperties) and
BrowserDynamicTestingModule/platformBrowserDynamicTesting as needed so the
Angular TestBed is initialized through setupZoneTestEnv; update imports to
reference setupZoneTestEnv and remove unused symbols (globalThis.ngJest,
TestBed.initTestEnvironment, setup-env/zone).
In `@gui-js/package.json`:
- Around line 169-176: Update the pinned `@types/jest` in package.json to the v30
range (e.g. ^30.0.0) and remove legacy setup-jest/ts-jest globals across the GUI
project; specifically, delete or stop importing "jest-preset-angular/setup-jest"
in gui-js/apps/minsky-electron/src/test-setup.ts and remove any
globals['ts-jest'] or ts-jest entries in gui-js/libs/menu/jest.config.ts (and
other jest.config.* files), then adopt the Jest-30 / jest-preset-angular-16
migration pattern by using setupZoneTestEnv() in your test setup and switching
to the preset/transform helpers recommended by the preset docs so runtime and
types are consistently using Jest 30.
- Around line 158-205: The engines.node field in package.json currently allows
">=20.11.1", which permits Node versions incompatible with Angular 21; update
the engines.node value to match Angular 21's supported range (e.g. "^20.19.0 ||
^22.12.0 || ^24.0.0") so that the package.json enforces only compatible Node
versions; locate and change the "engines": { "node": ... } entry in package.json
accordingly and run your usual install/CI to verify no version conflicts.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 837a9942-2ced-4111-a222-b64fd38ff841
⛔ Files ignored due to path filters (1)
gui-js/package-lock.jsonis excluded by!**/package-lock.json
📒 Files selected for processing (7)
gui-js/apps/minsky-web/src/app/app.component.tsgui-js/apps/minsky-web/src/main.tsgui-js/apps/minsky-web/src/test-setup.tsgui-js/apps/minsky-web/tsconfig.app.jsongui-js/apps/minsky-web/tsconfig.spec.jsongui-js/libs/shared/src/test-setup.tsgui-js/package.json
| async ngDoCheck() { | ||
| if(this.loading && this.router.url !== '/') { | ||
| if(this.loading) { | ||
| this.loading = false; | ||
| this.updatePubTabs(); | ||
| this.cdRef.detectChanges(); |
There was a problem hiding this comment.
Keep loading owned by the router.
At Line 60, loading is still cleared inside ngDoCheck, so the new NavigationEnd subscription at Lines 99-103 never actually controls the spinner. That means the loader still disappears on the first change-detection pass instead of after the first successful navigation.
Suggested fix
loading = true;
publicationTabs = [];
+ private initialized = false;
private windowUtilityService: WindowUtilityService;
private resizeTimeout;
@@
async ngDoCheck() {
- if(this.loading) {
- this.loading = false;
+ if(!this.initialized) {
+ this.initialized = true;
this.updatePubTabs();
this.cdRef.detectChanges();Keep this.loading = false; only in the NavigationEnd subscription.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@gui-js/apps/minsky-web/src/app/app.component.ts` around lines 58 - 62, The
ngDoCheck method currently clears this.loading too early, so remove the line
"this.loading = false;" from ngDoCheck and let the NavigationEnd subscription
(where you already set this.loading to false) be the single owner of the spinner
state; keep updatePubTabs() and this.cdRef.detectChanges() in ngDoCheck but
ensure only the NavigationEnd handler sets this.loading to false so the loader
closes after successful navigation (refer to ngDoCheck, updatePubTabs,
cdRef.detectChanges and the NavigationEnd subscription).
Removes @tootallnate/once. It's no longer used after updating ancestor dependencies @tootallnate/once, jest-environment-jsdom and jest-preset-angular. These dependencies need to be updated together.
Removes
@tootallnate/onceUpdates
jest-environment-jsdomfrom 29.7.0 to 30.2.0Release notes
Sourced from jest-environment-jsdom's releases.
... (truncated)
Changelog
Sourced from jest-environment-jsdom's changelog.
... (truncated)
Commits
855864ev30.2.0ebfa31cv30.1.2d347c0fv30.1.14d5f41dv30.1.022236cfv30.0.5f4296d2v30.0.4393acbfv30.0.25ce865bv30.0.1469f665v30.0.0ce14203v30.0.0-rc.1Updates
jest-preset-angularfrom 14.6.2 to 16.1.1Release notes
Sourced from jest-preset-angular's releases.
Changelog
Sourced from jest-preset-angular's changelog.
... (truncated)
Commits
0051cbdchore(release): 16.1.17010730fix: make snapshot serializer compatible with Signal Forms (#3572)f965bd2build(deps): update dependency globals to ^17.3.0343177abuild(deps): update dependency@angular/core[security]3425e70chore(release): 16.1.0d11d79dfeat: allow configuring platform providers forTestBedfrom setup envc2d55e6build(deps): update google/osv-scanner-action action to v2.3.35eddf95build(deps): update commitlint monorepo to ^20.4.25f783b7build(deps): update dependency glob to ^13.0.67e6b79bbuild(deps): update dependency zone.js to ~0.16.1Dependabot will resolve any conflicts with this PR as long as you don't alter it yourself. You can also trigger a rebase manually by commenting
@dependabot rebase.Dependabot commands and options
You can trigger Dependabot actions by commenting on this PR:
@dependabot rebasewill rebase this PR@dependabot recreatewill recreate this PR, overwriting any edits that have been made to it@dependabot show <dependency name> ignore conditionswill show all of the ignore conditions of the specified dependency@dependabot ignore this major versionwill close this PR and stop Dependabot creating any more for this major version (unless you reopen the PR or upgrade to it yourself)@dependabot ignore this minor versionwill close this PR and stop Dependabot creating any more for this minor version (unless you reopen the PR or upgrade to it yourself)@dependabot ignore this dependencywill close this PR and stop Dependabot creating any more for this dependency (unless you reopen the PR or upgrade to it yourself)You can disable automated security fix PRs for this repo from the Security Alerts page.
This change is
Summary by CodeRabbit
Improvements
Chores