Addon adoption plan #303
Replies: 4 comments 4 replies
-
Re-exports are no longer mandatory, but we're still in the process of actually getting rid of them and updating the documentation—I'm putting together a PR for part of that work right now 🙂 To your point, that was a huge blocker to addon adoption that is almost behind us, which makes this a great time to reassess the story for addons that wish to support Glint. The simplest answer is that addons "just" need to start declaring
That's right; the work that remains right now is largely around establishing a good set of conventions for addon authors and their consumers to follow for how to work with Glint-enabled addons. Beyond making proper use of signatures, the two main questions I see are:
For (1), the current thinking is to have addon authors declare a dependency on For (2), I think there are arguments to be made either way, with tradeoffs in ergonomics and flexibility depending on whether the addon itself or the app is responsible for putting additional entries in the template registry.
I think that's a great idea! I'd love to hear about any other issues you come across that we haven't considered. Thank you for being willing to experiment 🙂 |
Beta Was this translation helpful? Give feedback.
-
On the one hand, giving the app ownership of the template registry seems reasonable, as that's how things effectively work today, at least without strict mode and v1 addons with their On the other hand, this (overriding) is certainly the exception to the rule, like happening in <1% of cases probably? So forcing users to import and add things to the registry seems like a bad DX in the general case. Tbh I was puzzled a bit when I read this is what this But maybe we can allow both? On to my experiment first... I added Glint to a TS-based addon here: simonihmig/responsive-image#400 in the "usual" way, i.e. the template registry stuff was added in each module of a component or helper. The next step was to figure out how an app would be able to consume this. Interestingly the VSCode plugin seems to recognize this automatically: without any manual import it would show the correct component type in an app template. However that is (expectedly/correctly!?) not true when running glint CLI. I assume it needs to see an import of a module that features the template registry declaration to incorporate that, like any other tool that relies on static analysis of ES modules and which has no knowledge of the special build-time Emberisms, right? So in simonihmig/responsive-image#401 I added a glint.ts file which imports all the Glint-enabled modules (of the addon). Which I then imported from within the app's import '@glint/environment-ember-loose';
import '@glint/environment-ember-loose/native-integration';
import 'ember-responsive-image/glint'; This now made also Glint CLI happy. So the second use case of easily adding all the addon provided template registry entries seems to be covered, requiring just a single import from the app. Now back to making the app own the addon's template registry entries. The idea here is to not import that import type ResponsiveImage from 'ember-responsive-image/components/responsive-image';
declare module '@glint/environment-ember-loose/registry' {
export default interface Registry {
ResponsiveImage: typeof ResponsiveImage;
}
} On first glance this worked. However the entry above did not have any actual effect, as the template registry declaration of the component module ( import type ResponsiveImage from 'ember-responsive-image/components/responsive-image';
declare module '@glint/environment-ember-loose/registry' {
export default interface Registry {
ResponsiveImage: string; // just to trigger a type error
}
} This actually still works, Glint passes, the type of Tbh I was quite puzzled by that behavior, as I would have expected the "later" declaration to win. And TS to actually emit an error due to those incompatible shapes. But this was not the case! 🤔 So what did work to enable that use case was to move the template registry stuff from the actual component/helper modules to our Does that make sense so far? And is this dual mode approach something we should try to establish as an opinionated pattern for addons to adopt? (still room for bikeshedding the details) |
Beta Was this translation helpful? Give feedback.
-
Thank you for exploring and writing all this up, @simonihmig!
That's surprising to me, too—I would definitely have expected an error there! Is there any chance your app had
I like this approach a lot 👍 @chriskrycho @jamescdavis seem reasonable to you? |
Beta Was this translation helpful? Give feedback.
-
Oh yes, indeed, this is the case! Nice catch! |
Beta Was this translation helpful? Give feedback.
-
I wonder when and how addons should start adopting Glint, and what still needs to be done to do so?
The Readme still mentions it's too early for addons to adopt, but what is actually still too unstable? Re-exports are gone already, and other things like the template registry are going to stay as they are, aren't they?
I found out about
glint-template-types
too late (should have RTFM), but even with that a good amount of addons we use aren't covered. For those that I maintain myself, I would rather spend the time on making them come with their own types, rather than having to maintain un-synced ambient declarations. Or having those typed asany
-like, which I currently do, but that obviously defies the purpose...I guess addons can already use Glint internally for their own type safety without problems, right? So the question is more how to make them correctly publish their types, and how the app will consume them. I guess (without strict mode) the app would need to explicitly import them from somewhere, right?
If you don't have strong arguments against it, I'd be willing to experiment with some addon, to see how it goes. As long as it does not cause issues for non-Glint consumers, I would see Glint-support as an experimental feature for now, so wouldn't mind occasional breaking changes or rough edges...
Beta Was this translation helpful? Give feedback.
All reactions