RFC: Side-effect free addon registries

[dfreeman]

I'd like to explore a slight tweak to the conventions we prescribe for addons to contribute entries to the global template registry in loose-mode environments.

Current Design

In #303 and #313, several folks (with @simonihmig leading the charge!) put together an initial proposal for how addons should contribute to the template registry. That pattern has largely worked well, and has enabled addons to start shipping with Glint-native support (including a number of our internal packages at Salsify!)

At a high level, addons expose an $addon-name/glint module that, when imported, adds all their template entities to the global registry.

// addon/glint.ts
import CoolComponent from 'cool-addon/components/cool-component';
import coolHelper from 'cool-addon/helpers/cool-helper';

declare module '@glint/environment-ember-loose/registry' {
  export default interface Registry {
    CoolComponent: typeof CoolComponent;
    'cool-component': typeof CoolComponent;
    'cool-helper': typeof coolHelper;
  }
}

By providing this module at a dedicated import path, consumers can easily do a side-effect import of that module to automatically add all the addon's template entities to the registry:

import 'cool-addon/glint';
import myAppHelper from 'my-app/helpers/my-app-helper';

declare module '@glint/environment-ember-loose/registry' {
  export default interface Registry {
    'my-app-helper': typeof myAppHelper;
  }
}

This design also still gives consumers the option to instead import and add entries individually if it has its own local overrides or only wants to allow usage of a subset of an addon's template entities:

import coolHelper from 'cool-addon/helpers/cool-helper';
import myAppHelper from 'my-app/helpers/my-app-helper';

declare module '@glint/environment-ember-loose/registry' {
  export default interface Registry {
    'my-app-helper': typeof myAppHelper;
    'cool-helper': typeof coolHelper;
  }
}

Shortcomings

After about six months of real-world usage and battle testing, and as we start looking toward a 1.0 release, I'd like to propose one change to this convention. The place users have run into trouble is that adding the declare module statement implicitly makes the addon aware of @glint/environment-ember-loose and adds a required point of coordination between the addon and its host.

For that declaration to work, the addon must be able to "see" the host's copy of @glint/environment-ember-loose, which might not be the case if:

• the host is using a strict package manager and the addon hasn't declared a dependency on the environment package
• the host and the addon's resolved versions of the environment package are different
• the addon is linked or otherwise has the same version of the environment that's nevertheless located elsewhere on-disk from the host's copy

If any of those situations occur, the addon's entries won't be visible to the host in its own copy of the registry. In practice a number of folks have run into this and needed help debugging in Discord or here in the issue tracker.

Proposed Change

Rather than including a declare module statement themselves, addons could instead construct their own registry type representing the template entities they expose.

export interface CoolAddonRegistry {
  CoolComponent: typeof CoolComponent;
  'cool-component': typeof CoolComponent;
  'cool-helper': typeof coolHelper;
}

Consumers would then import that registry and extend the environment-ember-loose declaration of Registry to include those entries:

declare module '@glint/environment-ember-loose/registry' {
  export default interface Registry extends CoolAddonRegistry {
    'my-app-helper': typeof myAppHelper;
  }
}

You can see the rough shape of this in practice in this TS playground.

There are a few potential advantages to this convention (some more immediate and some more hypothetical):

• since addons never directly reference @glint/environment-ember-loose, resolution issues are no longer a problem
• if we move to e.g. @ember/glint-environment in the future, only the app's registry setup needs to change; addons will work without needing to be updates
• apps can more easily opt out of a single conflicting entry from an addon instead of having to opt in to everything but the conflict:

export default interface Registry extends Omit<CoolAddonRegistry, 'the-conflicting-one'> {
  'the-conflicting-one': SomeLocalType;
}

Open Questions

• Where should this registry type conventionally be exported from?
  • import { CoolAddonRegistry } from 'cool-addon';?
  • import { TemplateRegistry as CoolAddonRegistry } from 'cool-addon';? (maybe this is inject all over again)
  • import CoolAddonRegistry from 'cool-addon/template-registry';?
  • import CoolAddonRegistry from 'cool-addon/glint';?
  • Something else entirely?
  • What's the migration strategy for addons that currently expose a side-effect import at $addon-name/glint?
  • If we go with anything other than $addon-name/glint as the registry import path, then just keeping /glint around but deprecated seems like it should provide consumers with a reasonable path forward.

Thoughts from anyone who's worked with Glint in addons are welcome, but in particular I expect this is of interest to @chriskrycho @jamescdavis @simonihmig @NullVoxPopuli

[chriskrycho]

This makes really good sense to me—I have independently landed on the option of explicitly doing an interface merge like this which uses the interface A extends Q, R, S { ... } dynamic in a number of internal designs; it feels like the right balance of control and extensibility and convenience from the consumer’s POV.

In terms of the open questions, I like having it be import CoolAddonRegistry from 'cool-addon/glint'; best, and I think we could make that work in a backwards-compatible way:

• Introduce the registry as a default export and update your own registry definition there to be:

  export interface CoolAddonRegistry {
    CoolComponent: typeof CoolComponent;
    'cool-component': typeof CoolComponent;
    'cool-helper': typeof coolHelper;
  }
  
  declare module '@glint/environment-ember-loose/registry' {
    export default interface Registry extends CoolAddonRegistry {}
  }

• Cut a major release which removes the declare module '@glint/environment-ember-loose/registry' from cool-addon/glint.d.ts.

I believe that allows people to switch over incrementally; merging the same interface twice is totally legal (playground). So the consumer's migration path would be:

1. Update to the new minor of the addon.
2. Add the explicit extension in declare module '@glint/environment-ember-loose/registry' themselves.
3. Update to the new major of the addon.

(Also, there are definitely some addons doing this, but if we do this change quickly, there aren't that many—as you say, mostly internal!)

[dfreeman]

I believe that allows people to switch over incrementally; merging the same interface twice is totally legal

Merging a compatible type to the same interface twice is legal, but merging two incompatible types under the same field on the interface isn't. If you've got a local override for something in an addon, reusing cool-addon/glint means you can't adopt the new approach until after cool-addon has done its breaking release to drop the declare module. Maybe not a deal-breaker, but that was the one thing that gave me pause about reusing the existing /glint path.

[simonihmig]

I didn't run yet into the issues you described here myself, but the proposed change totally make sense for me, so I approve! :+1:

I like having it be import CoolAddonRegistry from 'cool-addon/glint'; best,

Same.

Merging a compatible type to the same interface twice is legal, but merging two incompatible types under the same field on the interface isn't.

Also, besides type conflicts, if you have imported the module with the registry side effect, you wouldn't be able to remove entries from it, right? Basically this use case Dan mentioned above would be impossible AFAIK:

or only wants to allow usage of a subset of an addon's template entities

FWIW, I have two addons (ember-stargate and ember-responsive-image) published with Glint support as it was designed before. In anticipation of things still being volatile, I explicitly called out that Glint support to be experimental and not covered by Semver (e.g. here. So I wouldn't really mind if this was a breaking change tbh. I have exactly one app where I have consumed these types myself, which is easy to fix, and I would be surprised if this was already used by a lot of other users, if by any at all. 🤷

[dfreeman]

In anticipation of things still being volatile, I explicitly called out that Glint support to be experimental and not covered by Semver

That's a good callout—I think every public addon I've seen so far with Glint support has gone that route. Since that's the case, and this is an easy break for consumers to absorb, I'm on board with sticking with my-addon/glint for the import path and letting authors do what makes sense from a versioning perspective 👍

[jamescdavis]

I'm definitely in favor of this! I'd like to vote for cool-addon/template-registry, though. If an addon is no longer adding to @glint/environment-ember-loose/registry, this file will no longer be specific to Glint. It will simply be a registry of everything provided by the addon that is invocable in a template. Maybe some other tool comes along that will use these in another way?

IMHO this reads better:

// import CoolAddon's template registry
import CoolAddonRegistry from 'cool-addon/template-registry';

declare module '@glint/environment-ember-loose/registry' {
  // merge it into Glint's registry
  export default interface Registry extends CoolAddonRegistry {}
}

I also like the aforementioned advantages:

• avoid issues with incompatible type merging
• immediately provide the ability to opt out of some invocables
• immediately provide a way to avoid side effects and resolution issues (while maintaining backwards compat)

But like others have said, those alone probably aren't enough to justify a new file. I just think, conceptually, we should have a new file that accurately describes what's in it (which now won't necessarily be specific Glint).

[simonihmig]

I'd like to vote for cool-addon/template-registry, though.

I can relate to James' arguments here, seems reasonable!

[chriskrycho]

Good points on the issues with /glint.d.ts—those weren’t immediately obvious to me, but they’re pretty important!—and James' note about /template-registry.d.ts make a lot of sense to me. Also, I noticed that the location becomes purely conventional—it doesn't have to exist at that location, even if most folks will put it there for consistency, and I take that to be a significant win.

Given that take, and summarizing @dfreeman's original proposal with that specific direction, we would end up with:

• Library code, conventionally (but not necessarily) at my-library/template-registry.d.ts:

  import Cool from 'my-library/components/cool';
  import FigureItOut from 'my-library/helpers/figure-it-out';
  
  export interface MyLibraryRegistry {
      Cool: typeof Cool;
      cool: typeof Cool;
      'figure-it-out': typeof FigureItOut;
  }

• Consuming app or library, literally anywhere (but probably most often in my-app/types/my-app/index.d.ts or similar):

  import 'ember-source/types';
  import 'ember-source/types/preview';
  import '@glint/environment-ember-loose';
  
  import { MyLibraryRegistry } from 'my-library/template-registry';
  
  declare module '@glint/environment-ember-loose/registry' {
    export default interface Registry extends MyLibraryRegistry {
      'my-app-helper': typeof myAppHelper;
    }
  }

This seems… pretty good!

[dfreeman]

👍 Sounds good to me!

All of this has only ever been convention, so we won't need to change anything about how Glint functions, "just" update the documentation and announce the change/encourage folks to migrate.