-
Notifications
You must be signed in to change notification settings - Fork 7
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
refactor: migrate addon to TypeScript #447
Conversation
bd0e7da
to
79c07da
Compare
79c07da
to
af0bd2c
Compare
Thanks @dannycalleri 🙏 I have one question/comment: I think we are missing the exportation of the types through the |
af0bd2c
to
17bccf9
Compare
I thought that each dependant could add it on its own, with an example to be added on the test app PR. But I like the approach we followed here: https://github.com/qonto/ember-amount-input/blob/cff4ba9306b553ce7ac66cbb549c3a4498c0ac84/ember-amount-input/src/template-registry.ts I added the same! |
With @m-jovan, we discussed introducing some jobs to lint the addon with different TypeScript versions. This way, we can offer a reliable TypeScript compatibility information. Here is an example in ember-amount-input. What do you think about this? Do you think we should consider adding the same here? |
I think it's a very good idea! 👍 Right now I see that we're specifying the working dir as I think that it would be beneficial to lint also the EDIT: this eventual change for the |
ff37d71
to
be47159
Compare
Yes, good idea 👍 In that case we'd need to introduce the same command ( |
8c24849
to
95b681a
Compare
f6ef3b5
to
cce2347
Compare
005b4b6
to
759be62
Compare
759be62
to
ad6baad
Compare
4232962
to
889b395
Compare
Just thought about this: I'd suggest to update the |
889b395
to
e4f31d8
Compare
e4f31d8
to
4f9510a
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
🚀
This PR migrates the addon code to TypeScript.
This safely removes the
{eager: false}
option introduced in ea239a3 to prepare forember-modifier@4
migration: https://github.com/ember-modifier/ember-modifier/blob/main/MIGRATIONS.md#when-is-this-a-breaking-changeNow that we're using v4, we can safely remove it since it's the default behavior. Also,
ember-modifier
types do not support that parameter anymore.