Skip to content

Refactor: Centralize calls to emberSafeRequire#2653

Merged
mansona merged 6 commits intoemberjs:mainfrom
mainmatter:centralized-glimmer-deps
Jun 19, 2025
Merged

Refactor: Centralize calls to emberSafeRequire#2653
mansona merged 6 commits intoemberjs:mainfrom
mainmatter:centralized-glimmer-deps

Conversation

@BlueCutOfficial
Copy link
Contributor

@BlueCutOfficial BlueCutOfficial commented Jun 11, 2025

Description

This PR is part of implementing Vite apps support for the inspector.

Inspecting an Ember app built with Vite or Ember CLI+Broccoli will change how the ember_debug.js script interacts with ember-source (specifically how Ember modules are imported). To implement with more confidence how ember_debug should adapt its behavior depending on that context, we started to centralize all the interactions with ember-source in one single script (ember_debug/utils/ember.js).

This PR continues this work by removing all the calls to emberSafeRequire over the different ember_debug files and replacing them with imports from ember_debug/utils/ember.js.

@BlueCutOfficial BlueCutOfficial force-pushed the centralized-glimmer-deps branch from 2c7bc8d to 3d23d20 Compare June 12, 2025 17:48
@BlueCutOfficial BlueCutOfficial force-pushed the centralized-glimmer-deps branch from 3d23d20 to 8135c6d Compare June 12, 2025 17:53
@BlueCutOfficial BlueCutOfficial force-pushed the centralized-glimmer-deps branch from d0f17f4 to 4dc681b Compare June 17, 2025 14:28
Comment on lines -20 to +29
try {
requireModule(
'@glimmer/manager',
).CustomModifierManager.prototype.getDebugInstance = (args) =>
args.modifier || args.delegate;
} catch {
// nope
if (GlimmerManager) {
GlimmerManager.CustomModifierManager.prototype.getDebugInstance = (
args,
) => args.modifier || args.delegate;
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

There are several places where I replaced try..catch with if. This is because in the initial implementation, requireModule could throw. With the new import from ember-debug/utils/ember, the method emberSafeRequire already handles the catch, so in this example GlimmerManager could just be undefined if the module is not found.

@BlueCutOfficial BlueCutOfficial marked this pull request as ready for review June 17, 2025 14:41
@BlueCutOfficial BlueCutOfficial changed the title Centralise calls to emberSafeRequire Centralize calls to emberSafeRequire Jun 17, 2025
@BlueCutOfficial BlueCutOfficial requested a review from mansona June 17, 2025 15:11
Copy link
Member

@mansona mansona left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

fantastic 🎉

@mansona mansona changed the title Centralize calls to emberSafeRequire Refactor: Centralize calls to emberSafeRequire Jun 19, 2025
@mansona mansona merged commit 51b07cb into emberjs:main Jun 19, 2025
21 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants