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

The latest published build breaks TypeScript apps #1035

Closed
andreyfel opened this issue Dec 5, 2023 · 6 comments · Fixed by #1037
Closed

The latest published build breaks TypeScript apps #1035

andreyfel opened this issue Dec 5, 2023 · 6 comments · Fixed by #1037
Labels

Comments

@andreyfel
Copy link
Contributor

Updating from 8.3.1 to 8.4.0 breaks linting in our app as it starts thinking that getOwner returns any.
The root cause of this are these comments added to published type declarations:

/// <reference types="ember-source/types/stable/@ember/owner/index" />
/// <reference types="ember-source/types/stable/@ember/controller/owner-ext" />
/// <reference types="ember-source/types/stable/@ember/routing/owner-ext" />
/// <reference types="ember-source/types/stable/@ember/service/owner-ext" />

https://www.npmjs.com/package/ember-file-upload/v/8.4.0?activeTab=code

Here is the relevant ticket in TypeScript repo:
microsoft/TypeScript#56571

Not sure what the addon authors should do about it.

  • Manually delete those /// comments before publishing?
  • Rollback Typescript to 5.2?
  • Wait until ember core team gives a recommendation?

I saw a discussion in Discord in dev-typescript channel but it hasn't resolved into any recommendations so far.

@andreyfel
Copy link
Contributor Author

andreyfel commented Dec 5, 2023

Recommendation from @NullVoxPopuli

If you're building the library, you can either downgrade typescript or use a post-build script to remove the triple slash reference lines before publish.

@gilest
Copy link
Collaborator

gilest commented Dec 7, 2023

Ahh change management in typescript is frustrating.

I was confident in releasing after this upgrade because we run type-checking in CI (for TS 5.0, 5.1, 5.2, 5.3). This issue does not fail those typechecks 😢

Any ideas how I can add regression coverage for this?

The Owner type is used in an internal modifier that's mounted in FileDropzone. But we have integration tests working out that component...

Do I need to import or use the Owner type elsewhere in the tests?

gilest added a commit that referenced this issue Dec 7, 2023
…ons #1037

Fixes #1035

Validated by manually checking the declarations/ output
@gilest
Copy link
Collaborator

gilest commented Dec 7, 2023

Rollback released in 8.4.1

@NullVoxPopuli
Copy link

Any ideas how I can add regression coverage for this?

add type-checking to the scenario testing

Do I need to import or use the Owner type elsewhere in the tests?

this is what ember-resources does: https://github.com/NullVoxPopuli/ember-resources/blob/f0a8b8506257add362e84dad49af8ec9218c8a66/ember-resources/src/core/function-based/manager.ts#L27-L37

Do I need to import or use the Owner type elsewhere in the tests?

you could possible cast to any or drop support for ember versions that don't have the Owner type.

@NullVoxPopuli
Copy link

Made a tool to help out in the future: https://github.com/NullVoxPopuli/fix-bad-declaration-output

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