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

Add type-tests #153

Open
wants to merge 1 commit into
base: main
Choose a base branch
from
Open

Add type-tests #153

wants to merge 1 commit into from

Conversation

NullVoxPopuli
Copy link
Collaborator

@NullVoxPopuli NullVoxPopuli commented Jul 15, 2023

Resolves #142 (<- also context)

Requires: #158

type-tests have become a pattern in TS-using v1 and v2 addons to help ensure that APIs maintain consistent and expected typings across TS upgrades.

the typed-ember team has recommended this approach on other v2 addons, and it is in used in ember-source.

@NullVoxPopuli NullVoxPopuli changed the base branch from main to add-fixture-utils July 15, 2023 21:16
@NullVoxPopuli NullVoxPopuli force-pushed the add-type-tests branch 2 times, most recently from 8f1e251 to e280e7a Compare July 15, 2023 22:43
@NullVoxPopuli NullVoxPopuli force-pushed the add-fixture-utils branch 3 times, most recently from 387847b to 5d2aae9 Compare July 17, 2023 12:27
Base automatically changed from add-fixture-utils to main July 17, 2023 12:31
@NullVoxPopuli NullVoxPopuli changed the base branch from main to typescript-compilation-tests July 17, 2023 19:41
@NullVoxPopuli NullVoxPopuli force-pushed the add-type-tests branch 2 times, most recently from a6fc51f to 7b3f383 Compare July 17, 2023 19:45
@NullVoxPopuli NullVoxPopuli marked this pull request as ready for review July 17, 2023 19:51
@NullVoxPopuli NullVoxPopuli added the enhancement New feature or request label Jul 18, 2023
@dfreeman
Copy link
Contributor

dfreeman commented Jul 18, 2023

If active members of the TS team have other opinions I'll happily defer to them, but my general take is that a typical TS addon shouldn't need type tests. In my experience, dedicated type tests are useful in two scenarios:

  • your addon and its tests are written in JS, and you want to verify that your hand-authored declarations work as expected
  • you're doing a significant amount of programming in the type system itself and want to "unit test" pieces of that logic at a very fine-grained level

For an addon authored in TypeScript, neither of these situations will usually apply, and I think it's generally going to be more productive to treat your actual runtime tests as effectively being end-to-end tests of your types.

Having good examples to point to for cases where authors do need type tests is great! But I don't think those cases are likely common enough that we should bake support into the default blueprint.

@simonihmig
Copy link
Collaborator

type-tests have become a pattern in TS-using v1 and v2 addons to help ensure that APIs maintain consistent and expected typings across TS upgrades.

the typed-ember team has recommended this approach on other v2 addons, and it is in used in ember-source.

I am curious, seems this is contrary to what Dan said above. Do you have examples for these type-tests in real addons?
And do they qualify as needed tests, in regard to the two scenarios Dan mentioned above?

@NullVoxPopuli
Copy link
Collaborator Author

Do you have examples for these type-tests in real addons?

ember-resources, ember-modifier, @ember/test-helpers, ember-async-data, ember-qunit, and ember-source (off the top of my head)

And do they qualify as needed tests, in regard to the two scenarios Dan mentioned above?

I think so, they each do goofy types-related things for various reasons.

  • types not the same as implementation
  • types jumping through hoops for Glint support

I am curious, seems this is contrary to what Dan said above.

Well, recommend for all and recommend for those that need them is probably the difference here.
For example, i wasn't using expect-type when i started type testing, and typed ember team was like, 'this tool is really good, you should use it'

Base automatically changed from typescript-compilation-tests to main July 19, 2023 13:56
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Add type-tests folder to addon folder when --typescript is enabled
3 participants