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

Publish Types? #877

Closed
NullVoxPopuli opened this issue Jun 4, 2020 · 5 comments
Closed

Publish Types? #877

NullVoxPopuli opened this issue Jun 4, 2020 · 5 comments

Comments

@NullVoxPopuli
Copy link
Collaborator

NullVoxPopuli commented Jun 4, 2020

the types over at @types/ember__test-helpers are a little wrong, and as with all separate types packages can be prone to fall out of sync -- not that ember-test-helpers has changed much recently.

Specifically, getContext() on @types/ember__test-helpers only returns an object, leading to type errors like this:


tests/helpers/get-service.ts:5:11 - error TS2339: Property 'owner' does not exist on type '{}'.

5   const { owner } = getContext();
            ~~~~~

tests/helpers/index.ts:38:11 - error TS2339: Property 'owner' does not exist on type '{}'.

38     let { owner } = getContext();
             ~~~~~

tests/helpers/setup-test.ts:27:18 - error TS2339: Property 'owner' does not exist on type 'object'.

27     getContext().owner.register('service:window', TestWindow);
                    ~~~~~

tests/helpers/stub-service.ts:19:9 - error TS2339: Property 'owner' does not exist on type '{}'.

19   let { owner } = getContext();
           ~~~~~

I've opened a PR here: DefinitelyTyped/DefinitelyTyped#45281
but I doubt it'll get merged, because

  • I don't have enough context / time :(
  • My one small fix probably isn't the correct fix, and I'm SUPER surprised no one has complained about this sooner?
  • Will probably need to change ember__test-helpers to import form ember__application to get the application / real owner. idk.
  • There are other places in ember__test-helpers that reference context that I didn't fix, because I wanted to focus on what I know.

This repo already has the types. I understand TS fragility, but even if separate type declarations were in this repo, that'd be a huge improvement.

@ro0gr
Copy link
Contributor

ro0gr commented Jun 4, 2020

regarding the owner. I believe the current recommended way is to use TestContext of the
"ember-test-helpers", which is hoisted from the "@types/ember-qunit".

Smth like:

import { TestContext as DefaultTestContext } from 'ember-test-helpers';
import { Something } from 'somewhere';

export interface TestContext extends DefaultTestContext {
  something: Something
}

module('my-module', funtion(hooks) {
  test('something', function(this: TestContext, assert) {
    this.owner.register('...');
    this.something = new Something();

    // ...
  });
})

@rwjblue
Copy link
Member

rwjblue commented Jun 5, 2020

First off, I would ABSOLUTELY LOVE to publish types, but frankly there are too many issues that are a bit uncharted with regards to providing TS types at the moment.

The work that @chriskrycho is doing over in typed-ember/ember-cli-typescript#1158 is the right path forward for us here, and once that lands we can discuss an Ember RFC to move things forward in official Ember repos.

Tldr; types will not be published by this package until we have figured out how to deal with the lack of ecosystem commitment to SemVer in the TypeScript space.

@NullVoxPopuli
Copy link
Collaborator Author

maybe the TS v4 series will introduce semver ❤️

@simonihmig
Copy link
Contributor

This can be closed?

@NullVoxPopuli
Copy link
Collaborator Author

sure can!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

4 participants