Inspector doesn't detect the app with `ember-source` v6.1 (under embroider)

[mkszepp]

Describe the bug
After updating my app to v6.1 (and also with new ember app), the inspector isn't working anymore.
The inspector doesn't detect the app.
With 6.0 it works without any issues

To Reproduce
ember new new-app --embroider --pnpm --typescript

Expected behavior
Inspector should find the app

Screenshots

Environment
Ember cli: 6.1
Ember version: 6.1
Embroider: latest
Browser: Chrome / Firefox

[patricklx]

since ember-source is now a v2 addon, no AMD modules are beeing created. So inspector cannot detect them. There is only a AMD compat mode for classic builds without embroider.
you have to include this code to make it work:

import Ember from 'ember';
import * as runtime from '@glimmer/runtime';
import * as tracking from '@glimmer/tracking';
import * as validator from '@glimmer/validator';
import { RSVP } from '@ember/-internals/runtime';

import config from './config/environment';


window.define('@glimmer/tracking', () => tracking);
window.define('@glimmer/runtime', () => runtime);
window.define('@glimmer/validator', () => validator);
window.define('rsvp', () => RSVP);
window.define('ember', () => ({ default: Ember }));
window.define('<my-app>/config/environment', () => ({
  default: config,
}));

[RobbieTheWagner]

@patricklx this does not seem ideal, is there any way we can fix this?

[esbanarango]

Same issue here.

"ember-source": "6.1.0",

[RobbieTheWagner]

@emberjs/framework This seems like a pretty critical issue we need a fix for.

[ef4]

To be clear, this is an issue under embroider builds on ember > 6.1. We were careful not to destabilize the default build and the inspector should work fine in a completely default ember 6.1 app.

But yes, the inspector makes assumptions about AMD that we can't possibly continue to support in an ES modules world.

[RobbieTheWagner]

Thanks for the context @ef4! Any thoughts on how we can support both going forward? It would be great if we could patch this and get some new versions out.

[ef4]

It would be helpful if somebody wants to clean up and centralize all the ways inspector is accessing modules from ember. I see require, requireModule, this.require, EmberLoader.require, and emberSafeRequire scattered throughout the ember_debug package.

It's also not clear to me whether the loader in inspector's ember_debug.js is allowed to interoperate with the app's own AMD loader. If yes, that needs to be untangled. It would probably be less confusing if ember_debug.js got built by Rollup instead of doing the custom-broccoli-pipeline-to-AMD thing. That would make it easier to see where there are real interactions with the app's modules vs the inspector's modules.

Another thing that could be helpful is if there are significant branches for dealing with older ember versions, let's do the snapshotting thing described by the readme and get them out of the maintained codebase, so we have less variety of Ember APIs to worry about replacing.

[chancancode]

I don't know to what extent things are broken right now, and to what extent they can be hacked to sort of work with a quick fix, but ultimately the current setup is unsustainable and needs to be completely flipped around.

There are a few category of things that the inspector (more specifically, ember_debug – the code that runs inside the same page as the app) is trying to access via loader.js:

(To be clear Ember.* is just an annotation here, assume they come from the appropriate ES module location)

• For convenience: grabbing stuff like utilities like isEmpty (just an example, haven't looked at the code to see what exactly). This is not good because if Ember stops shipping those utilities things will break. This is also the easiest to fix, either inline/vendor it, or send the raw data across the message channel and have the extension code do more of the heavy lifting.
• Access public Ember APIs: grabbing stuff like Ember.get or Ember.Object to subclass from. This is still bad because the app has enabled tree-shaking these may not be present. It's hard to say whether/which of these patterns are still really necessary. In theory, if Ember itself does a good job of not doing anything too out of the ordinary (use native getters and Proxies), most of that shouldn't be needed on this end either (assuming we drop the older versions as @ef4 suggested). However, there are probably still some cases where it is necessary to access the same kind of public APIs that the app should have access to – say, wanting to check someRuntimeValue instanceof Ember.Controller.
• Access private/intimate APIs: poking into the internals of Ember/Glimmer – to grab the application instance, detect the Ember version, validators stuff, debug render tree etc. This is the most brittle.

Historically, because the loader is present at runtime, very little consideration goes into vetting what functionalities should the inspector support; if someone can figure out how a hack by poking deep enough to get some useful information at runtime, then the feature gets added.

Ultimately I don't think that is very sustainable, and also the relationship needs to be flipped. The inspector needs to define the fundamental and relatively stable APIs it needs – say, "tell me the Ember version", "give me the application instance", "what routes are defined", "is this value a X", etc, and those needs to be bound, at runtime, to an runtime adapter provided by (and proactively registered by) the app bundle. It's not very different from, say, how libc works/how native apps can work across multiple operating system versions.

It's not that the application developers are supposed to implement any of those things in their app manually, but that should be indirected via a separately maintained layer, though some kind of plugin in the build may still be necessary to generate/inject some boilerplate into the bundle.

A possible version of this is:

1. Define an EmberDebug API. Say:

   interface EmberDebug {
     version: 1,
     getEmberVersion(): string;
     getProperty(obj: object, key: PropertyKey): unknown;
     // ...
   }

2. Define a protocol for connecting/registering with the inspector. For example, if the inspector is installed, it'll define a global $EmberInspector.attach(debug: EmberDebug): void; on the page, and something from the app bundle is expected to call it.
3. Create and maintain a package ember-debug that, when added to the application as a dependency, implements those APIs for you and does the registration with the inspector.

The last step is very hand-wavy, so additionally, here is one possible way to do that:

1. ember-debug is a package with ember-source as a peer-dependency (I am not sure if this is explicitly necessary/allowed/encouraged, but it is at least semantically what's happening). It should be tested with ember-try or similar to ensure things are working properly for the supported versions.
2. It can use @embroider/macros, conditional import(), among other things, to do what is necessary to implement those APIs.
3. Application developers need to put a tiny bit of boiler plate somewhere, maybe app.js, to do something along the lines of EmberDebug.init(app). It can take an options bag to opt-in/out of features so it doesn't pull in things that would have been tree-shaken out. It can also be wrapped in if (macroCondition(isDevelopingApp())) { or any other conditions to control whether or not the app should be inspectable.

I took a small stab at that years ago with someone at LinkedIn: https://github.com/emberjs/ember-debug. The constraints were different back then, and I think it was still written with the assumption that the Ember God-object has all the things in it. So the code itself is probably not super useful, but some structure may come in handy. I was also try to structure it such that EmberDebug is a somewhat sensible interface to use for debugging in general, outside of the Ember Inspector extension (e.g. if you know the API you can do $edb.askSomethingAboutMyApp()), which is also a nice to have.

(To be clear, I am not signing up to do this, but I think this is useful context for someone trying to understand the state of things and a potential path forward.)

[patricklx]

@ef4 yes, it can. But it's unnecessary now. I untangled it some time ago. So, no import is using ember and everything is using requireModule or other to access client ember.
But it's still here: https://github.com/emberjs/ember-inspector/blob/main/ember_debug/vendor/loader.js#L2
Uses global one if already exists.
There is a list of used modules here in this branch:

ember-inspector/ember_debug/utils/ember.js

Line 56 in b45f3b8

ActionHandler = emberSafeRequire('@ember/-internals/runtime')?.ActionHandler;

Though all mixin classes are only imported so that they instance can be checked and the name can be shown by inspector.

@chancancode there is already ongoing things.
We were reviewing this during office hours
emberjs/ember.js#20775

[RobbieTheWagner]

@ef4 We currently require Ember 3.16+ and versions before that get older versions of inspector. I was planning to require 3.28+ next but we could also make that something in the 4.x series if it is helpful.

@chancancode I totally agree that it would be ideal to flip the way inspector works. However, we now have a stable version of Ember out for which inspector does not work. My preference would be to find a way to fix this for folks now and free ourselves up to be able to do this major refactor at a slower pace.

[ef4]

In my previous comment I deliberately didn't go into the implementation details of how ember can provide the debug APIs, but we already have the infrastructure to support that and it's simpler than what is being discussed.

ember-source can already use dynamic import in every build environment. That means ember itself can contain code like:

globalThis.loadInspectorSupport = async () => {
  return await import("./the-stable-inspector-api");
}

Apps will not need any boilerplate and no macros are required. What will happen is your build tool (including ember-auto-import in classic builds) will emit an extra javascript chunk due to the presence of the above dynamic import, and that chunk will deploy alongside your other chunks, and the inspector can load it on demand.

So yes, there are two things to design. Godfrey is correct that we really really need to design a stable API. That would be the interface returned from loadInspectorSupport in my example.

And we should also design a reasonably flexible discovery mechanism. One that is insensitive to timing and that supports multiple ember apps simultaneously. A single global function like my example is too simple for reality.

[ef4]

I would add that Embroider can easily polyfill this feature on ember versions that lack it.

[RobbieTheWagner]

@ef4 I don't disagree, but I do think we need to prioritize this. Are you thinking we would patch ember-source back to 3.28 to do this or put in some conditional logic like if on 6.x do the new stuff?