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 TypeScript type definitions #565

Closed
wants to merge 3 commits into from

Conversation

charlesfries
Copy link
Contributor

Should fix #564

@NullVoxPopuli
Copy link
Contributor

any chance you can add type-checking to CI within the test-app?

the test-app could just have one TS file that is type-checked -- no need to convert the whole test-app to TS

@charlesfries
Copy link
Contributor Author

charlesfries commented Jul 15, 2024

@NullVoxPopuli One point of clarification--do you want me to import these definitions I've added in this PR into that TS file, or just convert some existing file to TS?

Also, I'm trying to figure out which file to convert to TS. Most files in test-app import Ember modules, which necessitates upgrading ember-source to a later version that includes the built in TS types. I can definitely do that, but I'm wondering if that should be a different PR entirely. Another option is just installing one or two of the DefinitelyTyped packages.

@NullVoxPopuli
Copy link
Contributor

it can be a standalone file -- no need to convert existing stuff.

like, you could have app/type-tests in the test-app, and have scenarios specifically for ensuring the APIs are correct -- no need to have runnable code as glint would verify that the types are correct.

, which necessitates upgrading ember-source to a later version that includes the built in TS types.

we could also do a separate project, maybe test-types (rather than test-app), which is the most minimal set of files we need to type check -- could even use built in types, and you don't need most of the files that come with an app

Copy link

netlify bot commented Jul 16, 2024

Deploy Preview for ember-sortable canceled.

Name Link
🔨 Latest commit 45da1dc
🔍 Latest deploy log https://app.netlify.com/sites/ember-sortable/deploys/66c4cc0ab168d1000814e9a8

@mkszepp
Copy link
Contributor

mkszepp commented Sep 20, 2024

converted the complete addon to TS #581 so this PR is not anymore needed... btw... the signatures here are not complety

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

Successfully merging this pull request may close these issues.

Addon doesn't supply TypeScript type definitions
3 participants