Registry is empty when passed to addons

[boris-petrov]

63 makes it so that when ember-cli calls setupRegistry here the registry argument is an empty object here. Which breaks some addons (for example ember-cli-styles). Is that expected?

cc @kategengler, @rwjblue, @2hu12

[kategengler]

I don't believe the registry argument is an empty object. The argument to setupRegistry is passed this, which has a registry set here. p is ember-cli-preprocess-registry/preprocessors, which returns a new Registry(). If registry was not there, no setupPreprocessorRegistry calls from any addon would be working.

Looking at the stack trace in that issue in ember-cli-styles, the issue is when ember-cli-styles-colocation tries to do registry.app.name see here. app was probably not intended to be a public property of Registry and was removed in that ember-cli-preprocessor-registry PR you linked as it was no longer used internally.

Oddly, the version of ember-cli-styles-colocation found on github does not use registry.app.name. The repo has two tags that are newer than the published version of 1.0.0-alpha.5, but also does not have a tag for that alpha.

Do you know which other addons are broken in the same way?

[boris-petrov]

The code you're looking for is here - it's a mono-repo of all the packages used by the addon.

I misspoke and you're right - registry is not an empty object but rather is not initialized the same way (which might be fine in most cases but I guess not for this one).

As for the other addons - I don't know, but a quick search in GH shows a few.

In any case - if that change is not going to be reverted, what are the migration steps?

[kategengler]

Maybe @ef4 can chime in on the future of setupPreprocessorRegistry and answer the question of if any addon should be looking for the app's name there

[boris-petrov]

Also note that at least ember-cli-styles looks for more than the app's name - for example here it uses registry.app.trees, registry.app.treePaths and registry.app.root. I don't know if all these could be changed to use something else - but in any case what are the cons of returning app to registry?

[kategengler]

I noticed those. Since they are all in index of an addon, you can use this._findHost to get the app. It is marked as private but extensively used such that we would probably have to treat it as public to make any changes. Alternatively, you could copy the implementation to a findApp on the index of an addon.

[ef4]

Maybe @ef4 can chime in on the future of setupPreprocessorRegistry

It doesn't have a future because it locks people into a broccoli-based build forever. I expect it to keep working for a long time within @embroider/compat but we're going to tell people to stop using it.

and answer the question of if any addon should be looking for the app's name there

It's not clear how anybody could have answered a "should" question like this when these things were so implementation-defined. So I don't want to litigate whether it was right or not, the only question is a practical one: how many addons does this break and how hard is it to fix them, vs restoring the old behavior.

[kategengler]

I don't believe many addons are broken and they can use this._findHost to get the app.

[boris-petrov]

@kategengler well, if that API is not going away any time soon, I guess it can be used instead. Unfortunately when I call it here, it returns undefined. Any idea why?

[kategengler]

@boris-petrov If you inspect this, does it look like the addon model?

[boris-petrov]

It looks so:

Class {
  _super: [Function: superFn] CoreObject {
    apply: [Function: apply],
    call: [Function: call],
    bind: [Function: bind],
    init: [Function: init]
  },
  parent: <ref *1> Project {...},
  project: <ref *1> Project {...},
  ui: UI ,
  addonPackages: [Object: null prototype] {},
  addons: [],
  registry: Registry { registry: {} },
  _nodeModulesPath: null,
  treePaths: {...},
  treeForMethods: {...},
  packageInfoCache: PackageInfoCache {...},
  _packageInfo: PackageInfo {...}
}

The this._findHost method is there, it just returns undefined.

Here is a simple repo that just installs that addon. Run npm install, edit node_modules/ember-cli-styles-colocation/index.js, add console.log(this._findHost()); on line 32 and run ember serve and you'll see undefined printed.

[boris-petrov]

OK, so for some reason, setupPreprocessorRegistry is called multiple times for the same addon. The first three times this._findHost() returns undefined but the fourth time it returns the correct thing. So I added:

const app = this._findHost();
if (app == null) {
  return;
}
registry.app = app;

In the beginning of setupPreprocessorRegistry. And that seems to work. Why does this happen? Is this "solution" OK?

[boris-petrov]

@kategengler, @ef4 - as you see in the PR we got it working and released so I can use ember-cli 5 now without an issue. Thanks for the support! Perhaps this issue can be closed. I'll let you decide if you want to do that or keep it open if you want to change something in this project here. Thanks!

[ef4]

OK, so for some reason, setupPreprocessorRegistry is called multiple times for the same addon.

It gets called once with the type argument equal to "parent" and once with type="self". One controls how the addon will preprocess its consumer's code and the other control how it will preprocess its own code.

All of that repeats per-consumption of the addon, so if the addon is used by the app and by two other addons, there will be three instances of the Addon class and setupPreprocessorRegistry will run six times.