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

@ember/test-helpers definition #237

Merged
merged 3 commits into from Jul 25, 2018

Conversation

Projects
None yet
3 participants
@happycollision
Copy link
Contributor

happycollision commented Jul 18, 2018

I mentioned this in Slack, and I figured that I ought to just do it and you can decide here if if works for you.

It adds a blueprint to create a definition for @ember/test-helpers in the types folder. It also adds a line about said file in the readme.

@chriskrycho

This comment has been minimized.

Copy link
Contributor

chriskrycho commented Jul 18, 2018

Let's get this on DefinitelyTyped! npm's search doesn't find it, but the package does actually exist.

@dfreeman

This comment has been minimized.

Copy link
Contributor

dfreeman commented Jul 18, 2018

For what it's worth, the typings exist in the npm registry too 😉
https://www.npmjs.com/package/@types/ember__test-helpers

@happycollision

This comment has been minimized.

Copy link
Contributor

happycollision commented Jul 18, 2018

Okay. So the way that npm does search and the way DT does search ember__test-helpers missing from TypeSearch means that people who don't know the idiosyncratic nature of a given package means that they cannot find it as easily...

Learning.

I'll adjust this PR to add the @types/ember__test-helpers to dependencies.

@happycollision happycollision force-pushed the happycollision:@ember-test-helpers branch from 954a1c2 to 1c0df1b Jul 18, 2018

@@ -95,6 +95,7 @@ In addition to ember-cli-typescript, we make the following changes to your proje
* **@types/rsvp** - ([npm](https://www.npmjs.com/package/@types/rsvp) | [source](https://github.com/DefinitelyTyped/DefinitelyTyped/tree/master/types/rsvp)) - Types for [RSVP.js](https://github.com/tildeio/rsvp.js/)
* **@types/ember-test-helpers** - ([npm](https://www.npmjs.com/package/@types/ember-test-helpers) | [source](https://github.com/DefinitelyTyped/DefinitelyTyped/tree/master/types/ember-test-helpers)) Types for [ember-test-helpers](https://github.com/emberjs/ember-test-helpers), which arose from [RFC #232](https://github.com/emberjs/rfcs/blob/master/text/0232-simplify-qunit-testing-api.md)
* **@types/ember-testing-helpers** - ([npm](https://www.npmjs.com/package/@types/ember-testing-helpers) | [source](https://github.com/DefinitelyTyped/DefinitelyTyped/tree/master/types/ember-testing-helpers)) – Types for [Ember's built-in globally-available test helpers](https://github.com/emberjs/ember.js/tree/master/packages/ember-testing/lib/helpers)
* **@types/ember__test-helpers** - ([npm](https://www.npmjs.com/package/@types/ember__test-helpers) | [source](https://github.com/DefinitelyTyped/DefinitelyTyped/tree/master/types/ember__test-helpers)) – Types for [ember-test-helpers](https://github.com/emberjs/ember-test-helpers) when imported as `@ember/test-helpers`.

This comment has been minimized.

@happycollision

happycollision Jul 18, 2018

Contributor

I am not sure that this is the best way to describe what is going on here. Because... I do not actually know what is going on here. Since this all comes from the same package that is already typed a few lines up above, I don't know why we need this definition, too. Shouldn't the other one declare both modules?

This comment has been minimized.

@dfreeman

dfreeman Jul 25, 2018

Contributor

There are a couple factors in play. One is that tools like VS Code can use import paths to provide intelligent typeahead and documentation even in plain JS files, but the apparent package name in the path needs to match up with the published @types package for that to work.

The other is that ember-test-helpers is actually no longer getting new releases — it's being published as @ember/test-helpers now, and that re-exports some old stuff (that's slated for deprecation, I believe) along with all the new helpers under the old ember-test-helpers namespace. Eventually, @ember/test-helpers will be the only thing left.

I think we probably want to move toward not adding @types/ember-test-helpers or @types/ember-testing-helpers to new projects, but that can happen as follow on work.

@happycollision

This comment has been minimized.

Copy link
Contributor

happycollision commented Jul 20, 2018

I am not sure why this isn't passing... though I only looked for a line that I thought was correct and added another one.

But I think this should actually do it, @chriskrycho.

@dfreeman

This comment has been minimized.

Copy link
Contributor

dfreeman commented Jul 25, 2018

Thanks @happycollision!

The previous failure looks like it was just a hiccup (albeit one in some fairly new testing infrastructure we have, so we should definitely keep a close eye on it in the future)

Okay. So the way that npm does search and the way DT does search ember__test-helpers missing from TypeSearch means that people who don't know the idiosyncratic nature of a given package means that they cannot find it as easily...

I actually didn't even know TypeSearch was a thing, but I see a number of open issues complaining that its behavior is weird and things seem to be missing.

Agreed that the naming scheme for scoped packages is odd and hard to discover, but it's a tricky scenario trying to invent a way to have a double-scoped package name. I'm not sure what layer of the tooling this comes from, but my editor at least suggests what @types package I should look for when it can't find declarations:

image

@chriskrycho chriskrycho merged commit cb3f7df into typed-ember:master Jul 25, 2018

2 checks passed

continuous-integration/appveyor/pr AppVeyor build succeeded
Details
continuous-integration/travis-ci/pr The Travis CI build passed
Details

dfreeman pushed a commit that referenced this pull request Aug 21, 2018

@happycollision happycollision deleted the happycollision:@ember-test-helpers branch Nov 21, 2018

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment