Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[Preview Types] Conflict with @types/ember__test-helpers #20254

Closed
boris-petrov opened this issue Nov 4, 2022 · 13 comments · Fixed by emberjs/ember-qunit#984
Closed

[Preview Types] Conflict with @types/ember__test-helpers #20254

boris-petrov opened this issue Nov 4, 2022 · 13 comments · Fixed by emberjs/ember-qunit#984
Labels
Bug TypeScript Work on Ember’s types

Comments

@boris-petrov
Copy link
Contributor

🐞 Describe the Bug

Trying to use the new preview-types instead of DefinitelyTyped ones doesn't work as there are errors in node_modules.

🔬 Minimal Reproduction

ember new preview-types-bug # make sure you use the latest `ember-cli` - 4.8.0
cd preview-types-bug
ember install ember-cli-typescript

Then follow the steps from the migration guide - i.e. remove the unneeded packages from package.json, run npm install, modify the preview-types-bug/types/preview-types-bug/index.d.ts file as shown there, and then run ./node_modules/.bin/tsc. You will get:

Found 111 errors in 25 files.

Errors  Files
    27  node_modules/@types/ember-data/index.d.ts:91
     5  node_modules/@types/ember/index.d.ts:87
     1  node_modules/@types/ember__application/index.d.ts:97
     2  node_modules/@types/ember__array/-private/enumerable.d.ts:163
     4  node_modules/@types/ember__array/-private/mutable-enumerable.d.ts:9
     7  node_modules/@types/ember__array/-private/native-array.d.ts:15
     4  node_modules/@types/ember__array/index.d.ts:19
     4  node_modules/@types/ember__array/mutable.d.ts:12
     1  node_modules/@types/ember__array/proxy.d.ts:6
     1  node_modules/@types/ember__component/-private/class-names-support.d.ts:24
     1  node_modules/@types/ember__component/-private/view-mixin.d.ts:55
     2  node_modules/@types/ember__controller/index.d.ts:52
     1  node_modules/@types/ember__engine/-private/container-proxy-mixin.d.ts:14
     1  node_modules/@types/ember__engine/-private/registry-proxy-mixin.d.ts:51
     1  node_modules/@types/ember__object/-private/action-handler.d.ts:27
     1  node_modules/@types/ember__object/-private/types.d.ts:75
    17  node_modules/@types/ember__object/core.d.ts:3
     1  node_modules/@types/ember__object/evented.d.ts:48
     7  node_modules/@types/ember__object/index.d.ts:13
     1  node_modules/@types/ember__object/internals.d.ts:1
    10  node_modules/@types/ember__object/mixin.d.ts:16
     4  node_modules/@types/ember__object/observable.d.ts:1
     1  node_modules/@types/ember__object/promise-proxy-mixin.d.ts:34
     3  node_modules/@types/ember__object/proxy.d.ts:3
     4  node_modules/@types/ember__service/index.d.ts:18

😕 Actual Behavior

A bunch of errors in node_modules.

🤔 Expected Behavior

No errors.

🌍 Environment

  • Ember: 4.8.2
  • Node.js/npm: Nodejs 19
  • OS: Arch Linux
  • Browser: N/A

➕ Additional Context

The problem is that @types/ember__test-helpers (which we shouldn't remove according to the guide) requires @types/ember__application which in turn requires @types/ember which then requires everything. And there are conflicts between the preview types and the DefinitelyTyped ones.

cc @chriskrycho, @wagenet

@chriskrycho
Copy link
Contributor

Thanks for the report. I'll dig in next week; this is interesting because I've never seen it in the tests I've done, so I'm not sure if something changed, or whether it's a knock-on effect from having the Ember Data @types packages present (which the docs do say are still an issue).

@chriskrycho chriskrycho added Bug TypeScript Work on Ember’s types labels Nov 5, 2022
@boris-petrov
Copy link
Contributor Author

boris-petrov commented Nov 5, 2022 via email

@chriskrycho chriskrycho changed the title [Preview Types] They do not work [Preview Types] Conflict with @types/ember__test-helpers Nov 9, 2022
@bertdeblock
Copy link
Member

Just wanted to confirm this issue. Happening for me as well.

@chriskrycho
Copy link
Contributor

Thanks for the further report! Hopefully I (or maaaaaybe @jamescdavis or @dfreeman) will have a chance to poke at it later this week!

@stukalin
Copy link
Contributor

Same here, just followed the guide...

Found 114 errors in 28 files.

Errors  Files
    27  node_modules/@types/ember-data/index.d.ts:91
     1  node_modules/@types/ember-resolver/index.d.ts:11
     5  node_modules/@types/ember/index.d.ts:87
     1  node_modules/@types/ember__application/index.d.ts:17
     2  node_modules/@types/ember__array/-private/enumerable.d.ts:163
     4  node_modules/@types/ember__array/-private/mutable-enumerable.d.ts:9
     7  node_modules/@types/ember__array/-private/native-array.d.ts:15
     4  node_modules/@types/ember__array/index.d.ts:19
     4  node_modules/@types/ember__array/mutable.d.ts:12
     1  node_modules/@types/ember__array/proxy.d.ts:6
     1  node_modules/@types/ember__component/-private/class-names-support.d.ts:24
     1  node_modules/@types/ember__component/-private/view-mixin.d.ts:55
     2  node_modules/@types/ember__controller/index.d.ts:52
     1  node_modules/@types/ember__debug/container-debug-adapter.d.ts:1
     1  node_modules/@types/ember__engine/-private/container-proxy-mixin.d.ts:14
     1  node_modules/@types/ember__engine/-private/registry-proxy-mixin.d.ts:51
     1  node_modules/@types/ember__engine/index.d.ts:15
     1  node_modules/@types/ember__object/-private/action-handler.d.ts:27
     1  node_modules/@types/ember__object/-private/types.d.ts:75
    17  node_modules/@types/ember__object/core.d.ts:3
     1  node_modules/@types/ember__object/evented.d.ts:48
     7  node_modules/@types/ember__object/index.d.ts:13
     1  node_modules/@types/ember__object/internals.d.ts:1
    10  node_modules/@types/ember__object/mixin.d.ts:16
     4  node_modules/@types/ember__object/observable.d.ts:1
     1  node_modules/@types/ember__object/promise-proxy-mixin.d.ts:34
     3  node_modules/@types/ember__object/proxy.d.ts:3
     4  node_modules/@types/ember__service/index.d.ts:18

@chriskrycho
Copy link
Contributor

In that case it's coming from having @types/ember-data, which the guide explicitly notes is not yet a supported flow. I'm planning to write up—at this point probably next week—a guide to doing for Ember Data the same as we did for Ember itself.

@gitKrystan
Copy link
Contributor

I am also running into a similar issue trying preview types, but I don't have @types/ember__test-helpers. I do have @types/ember-qunit. No other @types/ember* packages. The only type errors I get when running glint are from node_modules.

yarn why @types/ember__application
yarn why v1.22.19
[1/4] 🤔  Why do we have the module "@types/ember__application"...?
[2/4] 🚚  Initialising dependency graph...
[3/4] 🔍  Finding dependency...
[4/4] 🚡  Calculating file sizes...
=> Found "@types/[email protected]"
info Reasons this module exists
   - "@types#ember-qunit#@types#ember__test-helpers" depends on it
   - Hoisted from "@types#ember-qunit#@types#ember__test-helpers#@types#ember__application"
   - Hoisted from "@types#ember-qunit#@types#ember__test#@types#ember__application"
   - Hoisted from "@types#ember-qunit#@types#ember__test-helpers#@types#ember__application#@types#ember__application"
   - Hoisted from "@types#ember-qunit#@types#ember__test-helpers#@types#ember__application#@types#ember#@types#ember__application"

yarn why @types/ember__test-helpers
yarn why v1.22.19
[1/4] 🤔  Why do we have the module "@types/ember__test-helpers"...?
[2/4] 🚚  Initialising dependency graph...
[3/4] 🔍  Finding dependency...
[4/4] 🚡  Calculating file sizes...
=> Found "@types/[email protected]"
info Reasons this module exists
   - "@types#ember-qunit" depends on it
   - Hoisted from "@types#ember-qunit#@types#ember__test-helpers"
   - Hoisted from "@types#ember-qunit#@types#ember__test-helpers#@types#ember__test-helpers"

w/o @types/ember-qunit type checking for all my test files blows up.

@chriskrycho
Copy link
Contributor

Makes sense. Basically, we need to tackle the rest of #20162 to get these resolved. That's going to be my main focus for a bunch of the rest of this quarter, after getting #20234 done—hopefully next week, now that I’ve finished the work to implement RFC 0821, which we needed for the rest of this! That said, we (I!) could very much use the community’s help: there are a lot of packages which need types!

@wagenet
Copy link
Member

wagenet commented Nov 20, 2022

Spitballing here since I haven't actually spent the time to fully understand this situation:

Is it possible to make a custom package override for @types/ember__application that would resolve this issue?

@chriskrycho
Copy link
Contributor

Conceivably, but we don't want that: in some sense one major point of this precise period is to flush these things out and walk the ecosystem till they're solved!

@wagenet
Copy link
Member

wagenet commented Nov 23, 2022

@chriskrycho I guess I was thinking more in terms of a temporary workaround for individual apps.

@chriskrycho
Copy link
Contributor

Right, makes sense. Unfortunately I think any attempt at that would just make this problem worse, or at most equally bad. The problem is that the DT types are showing up transitively, so your dependencies are getting them, not you, so anything you do at the top level will just further confuse things.

@boris-petrov
Copy link
Contributor Author

I believe this issue has been resolved with the latest versions of ember-qunit, ember-resolver and @ember/test-helpers. Thanks for the hard work!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Bug TypeScript Work on Ember’s types
Projects
None yet
Development

Successfully merging a pull request may close this issue.

6 participants