[CLEANUP] Fix missing ApplicationInstance in Router tests#21095
[CLEANUP] Fix missing ApplicationInstance in Router tests#21095NullVoxPopuli merged 2 commits intoemberjs:mainfrom
Conversation
Estimated Asset SizesDiff --- main/out.txt 2026-02-16 14:34:00.000000000 +0000
+++ pr/./pr-22088232688/out.txt 2026-02-17 06:25:12.000000000 +0000
@@ -1,13 +1,13 @@
╔═══════╤═══════════╤═══════════╗
║ │ Min │ Gzip ║
╟───────┼───────────┼───────────╢
-║ Total │ 352.02 KB │ 203.83 KB ║
+║ Total │ 351.99 KB │ 203.84 KB ║
╚═══════╧═══════════╧═══════════╝
╔══════════════════════╤═══════════╤═══════════╗
║ @ember/* │ Min │ Gzip ║
╟──────────────────────┼───────────┼───────────╢
-║ Total │ 313.42 KB │ 181.89 KB ║
+║ Total │ 313.39 KB │ 181.91 KB ║
╟──────────────────────┼───────────┼───────────╢
║ -internals │ 36.65 KB │ 26.22 KB ║
║ application │ 13.23 KB │ 8.05 KB ║
@@ -25,7 +25,7 @@
║ object │ 35.94 KB │ 22.16 KB ║
║ owner │ 159 B │ 178 B ║
║ renderer │ 630 B │ 487 B ║
-║ routing │ 59.33 KB │ 34.1 KB ║
+║ routing │ 59.3 KB │ 34.12 KB ║
║ runloop │ 2.36 KB │ 1.5 KB ║
║ service │ 1 KB │ 845 B ║
║ template │ 654 B │ 541 B ║Details
|
|
@NullVoxPopuli Please Have a Review On this PR and Let me Know if this requires any Update or changes |
|
What error were you running in to that prompted looking in to this? |
|
@NullVoxPopuli I wasn't encountering a specific error in an application. I was looking through the codebase for cleanup opportunities and found the TODO(SAFETY) comment in packages/@ember/routing/router.ts. ApplicationInstance: |
|
Excellent! That certainly is helpful and i appreciate the effort! To ensure there is no regression in the future, could you add a test that fails before this change and passes after? (Or, if you remove the register do a bunch of tests fail? This would satisfy regression prevention as well) It's already a good signal that the whole test suite is currently passing already. |
|
@NullVoxPopuli i verified this locally. The changes to RouterNonApplicationTestCase |
|
There is no such thing as [CLEANUP beta] ... we do not backport Cleanup |
This PR removes a workaround in
packages/@ember/routing/router.tsthat allowedApplicationInstanceto be missing in tests. This occurred becauseRouterNonApplicationTestCaseinpackages/internal-test-helpersdid not register an application instance.I have updated the test case to register a mock
ApplicationInstance, allowing the router to enforce strict dependency checks and removing theTODO(SAFETY)comment.This ensures better type safety and correctness in the router's dependencies.