emberjs / ember.js

Ember.js - A JavaScript framework for creating ambitious web applications
https://emberjs.com
MIT License
22.48k stars 4.21k forks source link

`@glimmer/component` v2 is not working for ember < 4.10 #20790

Closed mkszepp closed 2 days ago

mkszepp commented 2 weeks ago

🐞 Describe the Bug

While whitelisting @glimmmer/component v2 in ember-basic-dropdown we have discoverd that isn't working in ember <= 4.8

The reason is that there was used @ember/owner inside @glimmer/components: https://github.com/emberjs/ember.js/blob/main/packages/%40glimmer/component/src/index.ts#L5

Error: Error: Could not find module@ember/ownerimported from ember-basic-dropdown/components/basic-dropdown

CI error: https://github.com/cibernox/ember-basic-dropdown/actions/runs/11777892189/job/32803253327?pr=941#step:6:202

The glimmer changelogs are telling ember < 3.13 is dropped see https://github.com/emberjs/ember.js/blob/main/packages/%40glimmer/component/CHANGELOG.md

kategengler commented 2 weeks ago

cc @ef4 I also think @glimmer/component is missing a README

ijlee2 commented 2 weeks ago

On a related note, after updating @glimmer/component from 1.1.2 to 2.0.0 in a project with Glint (with version 1.5.0 and typescript 5.6.3), I saw type errors in components with constructor():

import Component from '@glimmer/component';

interface MySignature {
  // ...
}

export default class MyComponent extends Component<MySignature> {
  constructor(owner: unknown, args: MySignature['Args']) {
    super(owner, args);

    // ...
  }
}
[lint:types] src/components/my-component.ts:31:11 - error TS2345: Argument of type 'unknown' is not assignable to parameter of type 'Owner'.
[lint:types] 
[lint:types] 31     super(owner, args);
[lint:types]              ~~~~~

I was under the impression that writing owner: unknown is okay, since owner is an implementation detail of Ember. I feel like I might have also picked up unknown from official documentations or some Ember repo(s). For example, https://typed-ember.gitbook.io/glint/troubleshooting/diagnosing-common-error-messages#does-not-satisfy-the-constraint-invokable.

Can you help us clarify whether we should write owner: Owner instead, where the type Owner comes from @ember/owner, or if this might be a bug introduced in 2.0.0?

mkszepp commented 2 weeks ago

we have seen the same error inside ember-power-select (1 usage was unknown)...

In additional i think we need a patch for current ember LTS versions, because @glimmer/component v2 is not whitelisted as peerDependency (i think in v6 it was already done)

ef4 commented 2 weeks ago

We should probably just document that @glimmer/component 2.0 has Ember 4.8 as its oldest supported version. Addons that want to support old Ember would also want to support both @glimmer/component 1.x and 2.x.

Owner instead of unknown is consistent with how most of the other extendable base classes in Ember work. All of these have Owner in their constructor type:

The Owner type was made to represent the public API of what an owner needs to have. It's distinct from Container for example, which would indeed be implementation detail. When that type was getting introduced throughout the codebase, it was also added to @glimmer/component but had been languishing in the unpublished 2.x betas.

mkszepp commented 2 weeks ago

@ef4 @ember/owner was introduced in 4.10

simonihmig commented 1 week ago

I can confirm, not possible to get this to work with Ember 4.8. See for example the linked PR above, all scenarios but 4.8 passing.

We should probably just document that @glimmer/component 2.0 has Ember 4.8 as its oldest supported version.

That needs to be 4.10 then, or 4.12 as a former LTS.